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

[4 Jul 2005 23:18] Domas Mituzas
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
[5 Jul 2005 7:25] Domas Mituzas
Or better... ROLLBACK transactions on COMMIT, not all, as read-only may happen just for fractions of seconds.
[7 Jul 2005 6:50] Domas Mituzas
just noting - master log pos changed, and data was visible on other sessions... :)
[24 Jul 2005 5:57] Heikki Tuuri
Changing the category: this is not an InnoDB bug but a general MySQL server bug.
--Heikki
[5 Aug 2005 11:35] Bugs System
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
[12 Sep 2006 20:06] Bugs System
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.
[11 Nov 2006 3:09] Marc ALFF
See related Bug#22009, and WL#3602
[11 Nov 2006 3:29] Marc ALFF
Manually changing to patch pending.
The patch is attached to Bug#22009
[15 Dec 2006 17:14] Marc ALFF
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.
[18 Jan 2007 15:53] Paul DuBois
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.
[18 Jan 2007 18:03] Marc ALFF
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.