[MDEV-11227] mysqlimport -l doesn't issue UNLOCK TABLES Created: 2016-11-03  Updated: 2016-12-22  Resolved: 2016-12-21

Status: Closed
Project: MariaDB Server
Component/s: Scripts & Clients
Affects Version/s: 10.2.3
Fix Version/s: 10.2.3

Type: Bug Priority: Blocker
Reporter: Sergey Vojtovich Assignee: Sergey Vojtovich
Resolution: Fixed Votes: 0
Labels: None

Issue Links:
Problem/Incident
is caused by MDEV-7660 MySQL WL#6671 "Improve scalability by... Closed
Sprint: 10.2.4-4, 10.2.4-1

 Description   

It wasn't a problem before MDEV-7660. With MDEV-7660 LOCK TABLES now starts transaction and this feature is generally broken, because transaction is rolled back on disconnect.



 Comments   
Comment by Elena Stepanova [ 2016-11-04 ]

I think mysqlimport is the least of our problems. Applications are not obliged to run UNLOCK TABLES explicitly! It's not even indeterministic behavior, it's documented and guaranteed that locks get released when the connection terminates:
http://dev.mysql.com/doc/refman/5.6/en/lock-tables.html

If the connection for a client session terminates, whether normally or abnormally, the server implicitly releases all table locks held by the session (transactional and nontransactional).

I bet that enormous number of existing applications rely on that.

What if a connection ends, for example, due to a wait timeout? It happens all the time. Or gets killed. Or... there are numerous situations where this change will break an existing logic.

svoj, serg, I beg you to re-consider. I see in the comments to MDEV-7660 that actually LOCK TABLE ... READ was a culprit that made you make this change. Then why not make only it disable auto-commit, while LOCK TABLE ... WRITE would keep it as is? It's a bit ugly, but it doesn't get uglier than it is now anyway. At the very first thought at least, with READ it won't have such dramatic effects; with WRITE it will get real bad real soon. Disconnect is only one problem. Think of replication, for instance. A connection can write-lock a table and write to it for hours, if not days, under assumption it auto-commits, and nothing will be written into binlog. And then, assuming it does issue UNLOCK, there will be an enormous multi-gig transaction?

Comment by Sergey Vojtovich [ 2016-11-04 ]

Well, one still isn't obligated to run UNLOCK TABLES. There's even almost no behaviour change when autocommit is unset initially.

Unfortunately one can issue LOCK TABLE t1 WRITE, t2 READ, that is in mixed case we'll have to disable autocommit too.

I have to leave it up to serg to defend this solution, because my intuition is not happy with current solution either.

Comment by Elena Stepanova [ 2016-11-04 ]

Well, one still isn't obligated to run UNLOCK TABLES.

What do you mean? It appears that all DML changes since the last LOCK TABLE .. WRITE will be lost if one does not run UNLOCK TABLES before disconnecting, isn't it the point of your bug report about mysqlimport?
With disabled autocommit – yes, there might be almost no change, but people don't disable autocommit all that massively, many (and maybe even the most) rely on explicit START TRANSACTION/COMMIT when they want a multi-statement transaction. And if the goal is to run only one statement, hardly anyone will do it.
We can request new applications to behave differently, but we cannot afford breaking compatibility with all existing applications in the world; and now we do, because without a thorough code inspection there is no guarantee that any application at all can use 10.2. And what's worse, it won't just break, it will just silently misbehave with consequences of unpredictable scale.

I don't see how an alleged improvement of scalability can be possibly worth it.

Comment by Elena Stepanova [ 2016-11-05 ]

Set to blocker and assigned to serg in hope it will be revisited before the release.

Comment by Sergei Golubchik [ 2016-11-08 ]

For backward compatibility we will implement a special handling for transactions implicitly opened by LOCK TABLES. These transactions will be committed on disconnect, not rolled back,

Comment by Sergey Vojtovich [ 2016-11-10 ]

Problem #1 (solvable, but confusing):

We have to consider autocommit setting that was before LOCK TABLES when deciding whether to commit or rollback transaction.

Disconnect must commit transaction:

SET @@autocommit=1;
LOCK TABLE t1 WRITE;

Disconnect must rollback transaction:

SET @@autocommit=0;
LOCK TABLE t1 WRITE;

The latter example is from http://dev.mysql.com/doc/refman/5.7/en/lock-tables-and-transactions.html

Comment by Sergey Vojtovich [ 2016-11-10 ]

Problem #2 (not completely related):

COMMIT now breaks lock:

CREATE TABLE t1(a INT) ENGINE=InnoDB;
SET @@autocommit=0;
LOCK TABLE t1 READ;
connect(con1, localhost, root);
send INSERT INTO t1 VALUES(1);
connection default;
COMMIT;
--sleep 1
SELECT * FROM t1;
UNLOCK TABLES;
connection con1;
reap;
disconnect con1;
connection default;
DROP TABLE t1;

Before MDEV-7660 SELECT didn't see "1", now it sees.
BUT if t1 is write locked, COMMIT doesn't break lock and thus "1" is not visible.

Comment by Elena Stepanova [ 2016-11-22 ]

Problem #2 extension (it might be unrelated to the disconnect issue, but it's definitely related to the root cause)

  • ROLLBACK now breaks lock;
  • statements which cause implicit commit can now break lock, e.g. ANALYZE TABLE !

Problem #3

This does not work anymore:

--source include/have_log_bin.inc
 
SET SESSION BINLOG_FORMAT=ROW;
CREATE TABLE t11 (song VARCHAR(255));
LOCK TABLES t11 WRITE;
INSERT INTO t11 VALUES('Several Species');
SET SESSION BINLOG_FORMAT=STATEMENT;
INSERT INTO t11 VALUES('Careful');
UNLOCK TABLES;

At line 7: query 'SET SESSION BINLOG_FORMAT=STATEMENT' failed: 1679: Cannot modify @@session.binlog_format inside a transaction

I'm sure there are many more surprises.

serg, again, given all the above, I beg to reconsider. Yes, I suppose every single issue can be solved by some cumbersome workaround, or declared as non-essential, but the sheer amount of incompatibilities that we introduce just does not make sense. Even if it were the only way to implement the desired optimization, it would still be very questionable, but to my understanding, svoj had an alternative solution which wasn't supposed to cause all of this, but it was discarded. Maybe it could be revived?

Comment by Sergey Vojtovich [ 2016-12-13 ]

serg, please review fix for this bug.

Generated at Thu Feb 08 07:48:17 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.