Details
Description
created after CONJ-948
Synchronous connectors can rely on OK_Packet/EOF_Packet server status's SERVER_STATUS_IN_TRANS flag that explicitly indicate if a transaction is currently active.
Java connector use that information to avoid issuing a rollback/commit command, skipping the command if there is no transaction.
Problem concerns only MyISAM that never set SERVER_STATUS_IN_TRANS flag. Still there is some case when there is some kind of lock : ( example from serg: )
create table t1 (a int) engine=myisam; |
|
|
insert t1 values (1); |
and in another
drop table t1; |
this DROP TABLE will wait until the transaction ends.
- either MyISAM should return SERVER_STATUS_IN_TRANS flag, so connector know there is really a commit to be issued.
- either MyISAM doesn't set any lock. (It may be just an issue : for some metadata locks are taken on the object name, before the table is opened, before we know it's a MyISAM table.)
Either way is ok.
(Another solution would be to always issue a COMMIT/ROLLBACK statement when there is no need, but that would degrade performance dramatically for some application. For example when connections are given back to pools, pools generally issues a connection.rollback() to ensure connection state, and there is lots of application where that command would represent 25% of the commands)
Attachments
Issue Links
- causes
-
CONJ-948 Under certain circumstances, Connector J does not send "commit"
-
- Closed
-
- relates to
-
-
- Confirmed
-
Activity
| Field | Original Value | New Value |
|---|---|---|
| Priority | Major [ 3 ] | Critical [ 2 ] |
| Affects Version/s | 10.3 [ 22126 ] | |
| Affects Version/s | 10.4 [ 22408 ] | |
| Affects Version/s | 10.5 [ 23123 ] | |
| Affects Version/s | 10.6 [ 24028 ] | |
| Affects Version/s | 10.7 [ 24805 ] | |
| Affects Version/s | 10.8 [ 26121 ] |
| Fix Version/s | 10.3 [ 22126 ] | |
| Fix Version/s | 10.4 [ 22408 ] | |
| Fix Version/s | 10.5 [ 23123 ] | |
| Fix Version/s | 10.6 [ 24028 ] | |
| Fix Version/s | 10.7 [ 24805 ] | |
| Fix Version/s | 10.8 [ 26121 ] |
| Assignee | Oleksandr Byelkin [ sanja ] |
| Description |
_created after Synchronous connectors can rely on OK_Packet/EOF_Packet server status's [SERVER_STATUS_IN_TRANS |https://mariadb.com/kb/en/ok_packet/#server-status-flag] flag that explicitly indicate if a transaction is currently active. Java connector use that information to avoid issuing a rollback/commit command, skipping the command if there is no transaction. Problem concerns only MyISAM that never set SERVER_STATUS_IN_TRANS flag. Still there is some case when there is some kind of lock : ( example from serg: ) {code:sql} create table t1 (a int) engine=myisam; start transaction; insert t1 values (1); {code} and in another {code:sql} drop table t1; {code} this {{DROP TABLE}} will wait until the transaction ends. * either MyISAM should return SERVER_STATUS_IN_TRANS flag, so connector know there is really a commit to be issued. * either MyISAM doesn't set any lock. (It may be just an issue : for some metadata locks are taken on the object *name*, before the table is opened, before we know it's a MyISAM table.) Either way is ok. (Another solution would be to always issue a COMMIT/ROLLBACK statement when there is no need, but that would degrade performance dramatically for some application. For example when connections are given back to pools, pools generally issues a connection.rollback() to ensure connection state, and there is lots of application where that command would represent 25% of the commands) |
_created after Synchronous connectors can rely on OK_Packet/EOF_Packet server status's [SERVER_STATUS_IN_TRANS |https://mariadb.com/kb/en/ok_packet/#server-status-flag] flag that explicitly indicate if a transaction is currently active. Java connector use that information to avoid issuing a rollback/commit command, skipping the command if there is no transaction. Problem concerns only MyISAM that never set SERVER_STATUS_IN_TRANS flag. Still there is some case when there is some kind of lock : ( example from serg: ) {code:sql} create table t1 (a int) engine=myisam; set autocommit=0; insert t1 values (1); {code} and in another {code:sql} drop table t1; {code} this {{DROP TABLE}} will wait until the transaction ends. * either MyISAM should return SERVER_STATUS_IN_TRANS flag, so connector know there is really a commit to be issued. * either MyISAM doesn't set any lock. (It may be just an issue : for some metadata locks are taken on the object *name*, before the table is opened, before we know it's a MyISAM table.) Either way is ok. (Another solution would be to always issue a COMMIT/ROLLBACK statement when there is no need, but that would degrade performance dramatically for some application. For example when connections are given back to pools, pools generally issues a connection.rollback() to ensure connection state, and there is lots of application where that command would represent 25% of the commands) |
| Summary | MyISAM wrong server status flags. | MyISAM wrong server status flags |
Try to fix which does not go well:
diff --git a/sql/mdl.cc b/sql/mdl.cc
|
index a53a1f08c2c..7f55dbf6fd4 100644
|
--- a/sql/mdl.cc
|
+++ b/sql/mdl.cc
|
@@ -1931,6 +1931,13 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request,
|
m_tickets[mdl_request->duration].push_front(ticket);
|
|
mdl_request->ticket= ticket;
|
+
|
+ if (mdl_request->duration == MDL_TRANSACTION &&
|
+ mdl_request->type == MDL_EXCLUSIVE)
|
+ {
|
+ m_owner->get_thd()->server_status|= SERVER_STATUS_IN_TRANS;
|
+ }
|
+
|
}
|
else
|
*out_ticket= ticket;
|
there are asserts which now fires and problem like that :
Cannot modify @@session.sql_log_bin inside a transaction
|
I updated time estimate (I have feeling we need to rewrite a lot to make it working as connector wants)
| Fix Version/s | 10.7 [ 24805 ] |
The fix require rewriting the status usage in the server, because the status flag (SERVER_STATUS_IN_TRANS) also taken into account by many server parts (like releasing transactional locks and so on), I am not sure such rewrite can be done in old versions.
| Status | Open [ 1 ] | In Progress [ 3 ] |
| Fix Version/s | 10.3 [ 22126 ] |
| Fix Version/s | 10.8 [ 26121 ] |
| Link | This issue relates to MDEV-30889 [ MDEV-30889 ] |
| Status | In Progress [ 3 ] | Stalled [ 10000 ] |
| Status | Stalled [ 10000 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | Stalled [ 10000 ] |
| Assignee | Oleksandr Byelkin [ sanja ] | Sergei Golubchik [ serg ] |
| Status | Stalled [ 10000 ] | In Progress [ 3 ] |
| Status | In Progress [ 3 ] | Stalled [ 10000 ] |
| Assignee | Sergei Golubchik [ serg ] | Oleksandr Byelkin [ sanja ] |
| Status | Stalled [ 10000 ] | In Review [ 10002 ] |
| Assignee | Oleksandr Byelkin [ sanja ] | Sergei Golubchik [ serg ] |
| Status | In Review [ 10002 ] | Stalled [ 10000 ] |
| Component/s | Protocol [ 14604 ] | |
| Fix Version/s | 10.4.32 [ 29300 ] | |
| Fix Version/s | 10.5.23 [ 29012 ] | |
| Fix Version/s | 10.6.16 [ 29014 ] | |
| Fix Version/s | 10.10.7 [ 29018 ] | |
| Fix Version/s | 10.11.6 [ 29020 ] | |
| Fix Version/s | 11.0.4 [ 29021 ] | |
| Fix Version/s | 11.1.3 [ 29023 ] | |
| Fix Version/s | 10.4 [ 22408 ] | |
| Fix Version/s | 10.5 [ 23123 ] | |
| Fix Version/s | 10.6 [ 24028 ] | |
| Resolution | Fixed [ 1 ] | |
| Status | Stalled [ 10000 ] | Closed [ 6 ] |
| Component/s | Locking [ 10900 ] |
| Component/s | Storage Engine - MyISAM [ 10600 ] |
| Description |
_created after Synchronous connectors can rely on OK_Packet/EOF_Packet server status's [SERVER_STATUS_IN_TRANS |https://mariadb.com/kb/en/ok_packet/#server-status-flag] flag that explicitly indicate if a transaction is currently active. Java connector use that information to avoid issuing a rollback/commit command, skipping the command if there is no transaction. Problem concerns only MyISAM that never set SERVER_STATUS_IN_TRANS flag. Still there is some case when there is some kind of lock : ( example from serg: ) {code:sql} create table t1 (a int) engine=myisam; set autocommit=0; insert t1 values (1); {code} and in another {code:sql} drop table t1; {code} this {{DROP TABLE}} will wait until the transaction ends. * either MyISAM should return SERVER_STATUS_IN_TRANS flag, so connector know there is really a commit to be issued. * either MyISAM doesn't set any lock. (It may be just an issue : for some metadata locks are taken on the object *name*, before the table is opened, before we know it's a MyISAM table.) Either way is ok. (Another solution would be to always issue a COMMIT/ROLLBACK statement when there is no need, but that would degrade performance dramatically for some application. For example when connections are given back to pools, pools generally issues a connection.rollback() to ensure connection state, and there is lots of application where that command would represent 25% of the commands) |
_created after Synchronous connectors can rely on OK_Packet/EOF_Packet server status's [SERVER_STATUS_IN_TRANS |https://mariadb.com/kb/en/ok_packet/#server-status-flag] flag that explicitly indicate if a transaction is currently active. Java connector use that information to avoid issuing a rollback/commit command, skipping the command if there is no transaction. Problem concerns only MyISAM that never set SERVER_STATUS_IN_TRANS flag. Still there is some case when there is some kind of lock : ( example from serg: ) {code:sql} create table t1 (a int) engine=myisam; insert t1 values (1); {code} and in another {code:sql} drop table t1; {code} this {{DROP TABLE}} will wait until the transaction ends. * either MyISAM should return SERVER_STATUS_IN_TRANS flag, so connector know there is really a commit to be issued. * either MyISAM doesn't set any lock. (It may be just an issue : for some metadata locks are taken on the object *name*, before the table is opened, before we know it's a MyISAM table.) Either way is ok. (Another solution would be to always issue a COMMIT/ROLLBACK statement when there is no need, but that would degrade performance dramatically for some application. For example when connections are given back to pools, pools generally issues a connection.rollback() to ensure connection state, and there is lots of application where that command would represent 25% of the commands) |
| Zendesk Related Tickets | 171314 |
I added reporting server status and it is what we have now:
create table t1 (a int) engine=innodb;set autocommit=0;--- INNODB start transactionstart transaction;affected rows: 0status: 1select * from t1;aaffected rows: 0status: 21insert t1 values (1);affected rows: 1status: 1select * from t1;a1affected rows: 1status: 21drop table t1;create table t1 (a int) engine=aria;commit;set autocommit=0;--- ARIA start transactionstart transaction;affected rows: 0status: 1select * from t1;aaffected rows: 0status: 1insert t1 values (1);affected rows: 1status: 1select * from t1;a1affected rows: 1status: 1drop table t1;create table t1 (a int) engine=innodb;commit;set autocommit=0;--- INNODB autmatic transactionselect * from t1;aaffected rows: 0status: 21insert t1 values (1);affected rows: 1status: 1select * from t1;a1affected rows: 1status: 21drop table t1;create table t1 (a int) engine=aria;commit;set autocommit=0;--- ARIA automatic transactionselect * from t1;aaffected rows: 0status: 0insert t1 values (1);affected rows: 1status: 0select * from t1;a1affected rows: 1status: 0drop table t1;test case:
--source include/have_innodb.inccreate table t1 (a int) engine=innodb;set autocommit=0;--echo --- INNODB start transaction--enable_infostart transaction;select * from t1;insert t1 values (1);select * from t1;--disable_infodrop table t1;create table t1 (a int) engine=aria;commit;set autocommit=0;--echo --- ARIA start transaction--enable_infostart transaction;select * from t1;insert t1 values (1);select * from t1;--disable_infodrop table t1;create table t1 (a int) engine=innodb;commit;set autocommit=0;--echo --- INNODB autmatic transaction--enable_infoselect * from t1;insert t1 values (1);select * from t1;--disable_infodrop table t1;create table t1 (a int) engine=aria;commit;set autocommit=0;--echo --- ARIA automatic transaction--enable_infoselect * from t1;insert t1 values (1);select * from t1;--disable_infodrop table t1;diff:
diff --git a/client/mysqltest.cc b/client/mysqltest.ccindex 2a6d4265e59..20b7762b0cf 100644--- a/client/mysqltest.cc+++ b/client/mysqltest.cc@@ -7729,11 +7729,13 @@ void append_metadata(DYNAMIC_STRING *ds,*/void append_info(DYNAMIC_STRING *ds, ulonglong affected_rows,- const char *info)+ const char *info, unsigned int status){char buf[40], buff2[21];sprintf(buf,"affected rows: %s\n", llstr(affected_rows, buff2));dynstr_append(ds, buf);+ sprintf(buf,"status: %x\n", status);+ dynstr_append(ds, buf);if (info){dynstr_append(ds, "info: ");@@ -7985,7 +7987,8 @@ void run_query_normal(struct st_connection *cn, struct st_command *command,query to find the warnings.*/if (!disable_info)- append_info(ds, mysql_affected_rows(mysql), mysql_info(mysql));+ append_info(ds, mysql_affected_rows(mysql), mysql_info(mysql),+ mysql->server_status);if (display_session_track_info)append_session_track_info(ds, mysql);@@ -8409,7 +8412,8 @@ void run_query_stmt(struct st_connection *cn, struct st_command *command,otherwise.*/if (!disable_info)- append_info(ds, mysql_stmt_affected_rows(stmt), mysql_info(mysql));+ append_info(ds, mysql_stmt_affected_rows(stmt), mysql_info(mysql),+ mysql->server_status);if (display_session_track_info)append_session_track_info(ds, mysql);