MySQL Bugs: #11733: COMMITs should not happen if read-only is set

Bug #11733	COMMITs should not happen if read-only is set
Submitted:	4 Jul 2005 23:18	Modified:	18 Jan 2007 18:03
Reporter:	Domas Mituzas	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: General	Severity:	S1 (Critical)
Version:	4.0, 4.1, 5.0	OS:	Any (Any)
Assigned to:	Marc ALFF	CPU Architecture:	Any

Description:
If read-only is set, INSERTs are forbidden, but COMMIT is not. Therefore even after read-only is set, data could be flushed to store and/or binlogs, which generally sucks and should not happen. 

How to repeat:
mysql> begin
    -> ;
Query OK, 0 rows affected (0.00 sec)
 
mysql> insert into ct values  ("a2");
Query OK, 1 row affected (0.00 sec)
-- OPERATOR TURNS ON READ ONLY FOR BACKUPS
 
mysql> insert into ct values  ("a3");
ERROR 1290 (HY000): The MySQL server is running with the --read-only option so it cannot execute this statement
mysql> commit;
Query OK, 0 rows affected (0.11 sec)

Suggested fix:
ROLLBACK all transactions in case of read-only

Or better... ROLLBACK transactions on COMMIT, not all, as read-only may happen just for fractions of seconds.

just noting - master log pos changed, and data was visible on other sessions... :)

Changing the category: this is not an InnoDB bug but a general MySQL server bug.
--Heikki

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/internals/27916

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/11799

ChangeSet@1.2263, 2006-09-12 14:06:28-07:00, malff@weblab.(none) +11 -0
  BUG#11733 (COMMITs should not happen if read-only is set)
  
  Before this patch,
  1) the COMMIT statement could write files even if the server is in
    --read-only mode, as reported.
  
  2) the server could reply to the "set global read_only=1;" query and
    *still* perform writes to the file system for a short (?) time,
    as found during investigation.
  
  3) the check to prevent statement execution in read-only mode was
    only a pre-check, and failed to detect cases when the server status
    changed *during* the statement execution.
    This is unsafe, and was found by code review.
    Depending of the timing and the duration of the statement (say,
    a full update on a very big table), this can be a real issue.
  
  With this patch,
  1) is fixed by having the ha_commit_trans() code for the 2 phase commit
  pay attention to the --read-only server status.
  
  2) is fixed by enforcing thread safety by protecting the *global*
    --read-only server status with a RW lock
  
  3) is fixed by 1). The pre-check code still exists to prevent up front
  most of the use cases, which as value since it notifies the client quickly.
  The code has been better commented.
  
  Fixing 1, 2 and 3 is important to guarantee that once the server has
  advertised itself as --read-only (by replying to the set global read_only=1
  query), files are actually safe to read when performing a file backup.
  
  Finally, a new replication test has been added to verify that:
  - the global variable read_only itself is not replicated,
  - setting read_only on the master replicates properly,
  - setting read_only on a slave cause the slave to fail, as expected.

See related Bug#22009, and WL#3602

Manually changing to patch pending.
The patch is attached to Bug#22009

Please note:

While it is correct that COMMITs should not happen if read_only is set,
which has now been fixed, I would like to elaborate on this comment in the original
bug report :

-- OPERATOR TURNS ON READ ONLY FOR BACKUPS

The read_only option is documented as :
  --read_only         Make all non-temporary tables read-only, with the
                      exception for replication (slave) threads and users with
                      the SUPER privilege

Using SET GLOBAL READ_ONLY=1 alone does not prevent *all* write operations,
only most of them.
To safely perform a backup, the system administrator also should stop the
slaves in case of replication (if doing a backup of a slave server),
and not execute queries under the SUPER privilege during the backup.

Thanks for the report.

Noted in 5.1.15 changelog.

Incompatible change: The following conditions apply to
enabling the read_only system variable:

  + If you attempt to enable read_only while you have
    any explicit locks (acquired with LOCK TABLES or
    have a pending transaction, an error will occur.

  + If other clients hold explicit table locks or have
    pending transactions, the attempt to enable
    read_only blocks until the locks are released and 
    the transactions end. While the attempt to enable
    read_only is pending, requests by other clients for 
    table locks or to begin transactions also block
    until read_only has been set.

  + read_only can be enabled while you hold a global
    read lock (acquired with FLUSH TABLES WITH READ
    LOCK) because that does not involve table locks.
    This means that the following sequence of statements
    can be used to place a server in read-only state
    prior to performing a backup:

FLUSH TABLES WITH READ LOCK;
SET GLOBAL read_only = ON;
UNLOCK TABLES;

Previously, the attempt to enable read_only would return
immediately even if explicit locks or transactions were
pending, so some data changes could occur for statements
executing in the server at the same time.

I am returning the report to Patch Pending status in case the
change will also be made to 5.0. If not, please close the report.

This fix introduce some incompatible changes in the behavior of
SET GLOBAL READ_ONLY, which now can block instead of returning immediately.

As a result, this fix is currently for 5.1 only, and is not planned for 5.0.
Closing the report, please re-open if this needs to be re-evaluated.