[MDEV-34589] Assertion `!is_set() || (m_status == DA_EOF_BULK && is_bulk_op())' failed from mysql_admin_table on CHECK TABLE - Jira

Details

Type: Bug
Status: Closed (View Workflow)
Priority: Critical
Resolution: Fixed
Affects Version/s: 10.5, 10.6, 10.11, 11.1(EOL), 11.2(EOL), 11.4, 11.5(EOL), 11.6(EOL)
Fix Version/s: 10.5.27, 10.6.20, 10.11.10, 11.2.6, 11.4.4
Component/s: Authentication and Privilege System, Partitioning, Storage Engine - Spider
Labels:
- privileges

Description

INSTALL PLUGIN Spider SONAME 'ha_spider.so';

SET spider_internal_sql_log_off=0;

CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET '../socket.sock', DATABASE 'test', USER 'Spider', PASSWORD '');

CREATE TABLE t1 (c INT) ENGINE=InnoDB;

CREATE TABLE t2 (c INT) ENGINE=Spider COMMENT='WRAPPER "mysql", SRV "srv", TABLE "t1"' PARTITION BY HASH (c) PARTITIONS 2;

CHECK TABLE t2;

Leads to:

11.2.5 a21e49cbcc5f4adb1a1b4970ceead6a85e968063 (Debug)
mariadbd: /test/11.2_dbg/sql/sql_error.cc:375: void Diagnostics_area::set_eof_status(THD*): Assertion `!is_set() \|\| (m_status == DA_EOF_BULK && is_bulk_op())' failed.

11.2.5 a21e49cbcc5f4adb1a1b4970ceead6a85e968063 (Debug)
Core was generated by `/test/MD190624-mariadb-11.2.5-linux-x86_64-dbg/bin/mariadbd --no-defaults --max'.
Program terminated with signal SIGABRT, Aborted.
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>)at ./nptl/pthread_kill.c:44
Download failed: Invalid argument. Continuing without source file ./nptl/./nptl/pthread_kill.c.
[Current thread is 1 (LWP 2037238)]
(gdb) bt
#0 __pthread_kill_implementation (no_tid=0, signo=6, threadid=<optimized out>)at ./nptl/pthread_kill.c:44
#1 __pthread_kill_internal (signo=6, threadid=<optimized out>)at ./nptl/pthread_kill.c:78
#2 __GI___pthread_kill (threadid=<optimized out>, signo=signo@entry=6)at ./nptl/pthread_kill.c:89
#3 0x0000151852a42866 in __GI_raise (sig=sig@entry=6)at ../sysdeps/posix/raise.c:26
#4 0x0000151852a268b7 in __GI_abort () at ./stdlib/abort.c:79
#5 0x0000151852a267db in __assert_fail_base (fmt=0x151852bc5168 "%s%s%s:%u: %s%sAssertion `%s' failed.\n%n", assertion=assertion@entry=0x562141314560 "!is_set() \|\| (m_status == DA_EOF_BULK && is_bulk_op())", file=file@entry=0x5621413143c0 "/test/11.2_dbg/sql/sql_error.cc", line=line@entry=375, function=function@entry=0x562141314598 "void Diagnostics_area::set_eof_status(THD*)") at ./assert/assert.c:92
#6 0x0000151852a39186 in __assert_fail (assertion=0x562141314560 "!is_set() \|\| (m_status == DA_EOF_BULK && is_bulk_op())", file=0x5621413143c0 "/test/11.2_dbg/sql/sql_error.cc", line=375, function=0x562141314598 "void Diagnostics_area::set_eof_status(THD*)")at ./assert/assert.c:101
#7 0x00005621405a819d in Diagnostics_area::set_eof_status (this=0x151810006f40, thd=thd@entry=0x151810000d58)at /test/11.2_dbg/sql/sql_error.cc:375
#8 0x000056214077a93f in my_eof (thd=0x151810000d58)at /test/11.2_dbg/sql/sql_class.h:4535
#9 mysql_admin_table (thd=thd@entry=0x151810000d58, tables=tables@entry=0x1518100136e8, check_opt=check_opt@entry=0x1518100065c8, operator_name=operator_name@entry=0x562141b8dfe0 <msg_check>, lock_type=lock_type@entry=TL_READ_NO_INSERT, org_open_for_modify=org_open_for_modify@entry=false, repair_table_use_frm=<optimized out>, extra_open_options=<optimized out>, prepare_func=<optimized out>, operator_func=<optimized out>, view_operator_func=<optimized out>, is_cmd_replicated=<optimized out>)at /test/11.2_dbg/sql/sql_admin.cc:1455
#10 0x000056214077b08e in Sql_cmd_check_table::execute (this=<optimized out>, thd=0x151810000d58) at /test/11.2_dbg/sql/sql_admin.cc:1588
#11 0x00005621405f9670 in mysql_execute_command (thd=thd@entry=0x151810000d58, is_called_from_prepared_stmt=is_called_from_prepared_stmt@entry=false)at /test/11.2_dbg/sql/sql_parse.cc:5876
#12 0x00005621405fb010 in mysql_parse (thd=thd@entry=0x151810000d58, rawbuf=<optimized out>, length=<optimized out>, parser_state=parser_state@entry=0x15183c1af2e0)at /test/11.2_dbg/sql/sql_parse.cc:7920
#13 0x00005621405fd3d3 in dispatch_command (command=command@entry=COM_QUERY, thd=thd@entry=0x151810000d58, packet=packet@entry=0x15181000b2f9 "", packet_length=packet_length@entry=14, blocking=blocking@entry=true)at /test/11.2_dbg/sql/sql_class.h:247
#14 0x00005621405ff76c in do_command (thd=0x151810000d58, blocking=blocking@entry=true) at /test/11.2_dbg/sql/sql_parse.cc:1407
#15 0x0000562140766c49 in do_handle_one_connection (connect=<optimized out>, connect@entry=0x562144cfb518, put_in_cache=put_in_cache@entry=true)at /test/11.2_dbg/sql/sql_connect.cc:1439
#16 0x0000562140766f3e in handle_one_connection (arg=arg@entry=0x562144cfb518)at /test/11.2_dbg/sql/sql_connect.cc:1341
#17 0x0000562140bb952c in pfs_spawn_thread (arg=0x562144c8f488)at /test/11.2_dbg/storage/perfschema/pfs.cc:2201
#18 0x0000151852a97ada in start_thread (arg=<optimized out>)at ./nptl/pthread_create.c:444
#19 0x0000151852b2847c in clone3 ()at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:78

Somewhat concerning, on optimized builds we see (also see MDEV-33196):

10.5.26 6cab2f75fe25df4673cab5ac6ff2c114fb976ed8 (Optimized)
10.5.26-opt>CHECK TABLE t2;
ERROR 1227 (42000): Access denied; you need (at least one of) the SUPER privilege(s) for this operation

Bug confirmed present in 10.5+.

Attachments

Issue Links

relates to

MDEV-34064 ERROR 12607 (HY000): This xid is not exist and Assertion `!is_set() || (m_status == DA_OK_BULK && is_bulk_op())' failed from change_master()

Confirmed

MDEV-33196 [ERROR] Got error 12524 when reading table './test/t'

Open

Activity

Ascending order - Click to sort in descending order

Roel Van de Paar added a comment - 2024-07-14 23:20

MTR testcase:

--source include/have_innodb.inc

--source include/have_partition.inc

--let $SOCKET=`SELECT @@global.socket`

INSTALL PLUGIN Spider SONAME 'ha_spider.so';

CREATE USER spider@localhost IDENTIFIED BY 'pwd';

GRANT ALL ON test.* TO spider@localhost;

eval CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$SOCKET",DATABASE 'test',USER 'spider',PASSWORD 'pwd');

SET spider_same_server_link=1;

SET spider_internal_sql_log_off=0;

CREATE TABLE t1 (c INT) ENGINE=InnoDB;

CREATE TABLE t2 (c INT) ENGINE=Spider COMMENT='WRAPPER "mysql", SRV "srv", TABLE "t1"' PARTITION BY HASH (c) PARTITIONS 2;

CHECK TABLE t2;

Roel Van de Paar added a comment - 2024-07-14 23:20 MTR testcase: --source include/have_innodb.inc --source include/have_partition.inc --let $SOCKET=`SELECT @@global.socket` INSTALL PLUGIN Spider SONAME 'ha_spider.so' ; CREATE USER spider@localhost IDENTIFIED BY 'pwd' ; GRANT ALL ON test.* TO spider@localhost; eval CREATE SERVER srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$SOCKET" , DATABASE 'test' , USER 'spider' , PASSWORD 'pwd' ); SET spider_same_server_link=1; SET spider_internal_sql_log_off=0; CREATE TABLE t1 (c INT ) ENGINE=InnoDB; CREATE TABLE t2 (c INT ) ENGINE=Spider COMMENT= 'WRAPPER "mysql", SRV "srv", TABLE "t1"' PARTITION BY HASH (c) PARTITIONS 2; CHECK TABLE t2;

Roel Van de Paar added a comment - 2024-07-14 23:28

Confirmed to still be present as described in 10.5 trunk as of today @ 3c508d4c71c8bf27c6ecceba53ab9d4325a1bd6c.

Roel Van de Paar added a comment - 2024-07-14 23:28 Confirmed to still be present as described in 10.5 trunk as of today @ 3c508d4c71c8bf27c6ecceba53ab9d4325a1bd6c .

Yuchen Pei added a comment - 2024-07-16 03:01 - edited

With an explicit SET spider_internal_sql_log_off=0;, spider will attempt to set sql_log_off at the data node when connecting to the data node, which results in a 1227 ER_SPECIFIC_ACCESS_DENIED_ERROR failure. This happens in both the rnd_next() call and the rollback call mentioned below.

This issue is an example of unclear contracts in the codebase, which indicates several possibilities for a fix.

1. The sql layer function implementing the logic of CHECK TABLE is mysql_admin_table(). It calls the ha_analyze() method on the storage engine handler, and handles the return in a switch. It seems to expect the return to be one of the several HA_ADMIN_* codes, and any other code is deemed a fatal error, which would cause a rollback. The storage engine in the testcase is the partition one, which not only calls ha_optimize() on the handlers of its partitions (in ha_partition::handle_opt_part()), but also the ha_rnd_*() methods (in ha_partition::check_misplaced_rows()). It passes back any error from these functions except HA_ERR_END_OF_FILE to mysql_admin_table(). The specific error in spider is not fatal and should result in a HA_ADMIN_FAILED. The partition engine would not know how to convert arbitrary errors returned from ha_rnd_*() to an HA_ADMIN_* error. Therefore one fix could be for spider to check for the sql command in ha_rnd_next() and return HA_ADMIN_FAILED if it encounters the 1227 error. But this seems like a very hacky and ad-hoc solution.
2. In ha_rollback_trans(), there's code comment saying handlerton rollback cannot fail. Why so? Does this mean handlerton::rollback() should always return 0? If so then the fix should be clearing errors in spider handlerton rollback.
3. The function trans_rollback_stmt() has code documentation that it may return FALSE (success) or TRUE (failure), but the function body always returns FALSE. Further, ha_rollback_trans() called in this function may return error, but its return value is ignored which causes the inconsistency in dastatus and assertion error. Therefore another possibility is to honour failure returned by ha_rollback_trans() by returning TRUE.

Here's an example solution number 2:

a9a064543c8 bb-10.5-mdev-34589-restore-dastatus-rollback MDEV-34589 spider: unconditionally restore da status in rollback

e6eb0629a5e MDEV-34589 Update signature of trans_rollback_stmt() to return void

Here's a solution number 3:

df05e3115d8 upstream/bb-10.5-mdev-34589-trans-rollback-pass-error bb-10.5-ycp MDEV-34589 pass error from ha_rollback_trans() to trans_rollback_stmt()

Solution 3 is the cleanest, so I would go with this one.

serg: let me know if you have any thoughts on this.

Yuchen Pei added a comment - 2024-07-16 03:01 - edited With an explicit SET spider_internal_sql_log_off=0; , spider will attempt to set sql_log_off at the data node when connecting to the data node, which results in a 1227 ER_SPECIFIC_ACCESS_DENIED_ERROR failure. This happens in both the rnd_next() call and the rollback call mentioned below. This issue is an example of unclear contracts in the codebase, which indicates several possibilities for a fix. 1. The sql layer function implementing the logic of CHECK TABLE is mysql_admin_table() . It calls the ha_analyze() method on the storage engine handler, and handles the return in a switch. It seems to expect the return to be one of the several HA_ADMIN_* codes, and any other code is deemed a fatal error, which would cause a rollback. The storage engine in the testcase is the partition one, which not only calls ha_optimize() on the handlers of its partitions (in ha_partition::handle_opt_part() ), but also the ha_rnd_*() methods (in ha_partition::check_misplaced_rows() ). It passes back any error from these functions except HA_ERR_END_OF_FILE to mysql_admin_table() . The specific error in spider is not fatal and should result in a HA_ADMIN_FAILED . The partition engine would not know how to convert arbitrary errors returned from ha_rnd_*() to an HA_ADMIN_* error. Therefore one fix could be for spider to check for the sql command in ha_rnd_next() and return HA_ADMIN_FAILED if it encounters the 1227 error. But this seems like a very hacky and ad-hoc solution. 2. In ha_rollback_trans() , there's code comment saying handlerton rollback cannot fail. Why so? Does this mean handlerton::rollback() should always return 0? If so then the fix should be clearing errors in spider handlerton rollback. 3. The function trans_rollback_stmt() has code documentation that it may return FALSE (success) or TRUE (failure), but the function body always returns FALSE. Further, ha_rollback_trans() called in this function may return error, but its return value is ignored which causes the inconsistency in dastatus and assertion error. Therefore another possibility is to honour failure returned by ha_rollback_trans() by returning TRUE. Here's an example solution number 2: a9a064543c8 bb-10.5-mdev-34589-restore-dastatus-rollback MDEV-34589 spider: unconditionally restore da status in rollback e6eb0629a5e MDEV-34589 Update signature of trans_rollback_stmt() to return void Here's a solution number 3: df05e3115d8 upstream/bb-10.5-mdev-34589-trans-rollback-pass-error bb-10.5-ycp MDEV-34589 pass error from ha_rollback_trans() to trans_rollback_stmt() Solution 3 is the cleanest, so I would go with this one. serg : let me know if you have any thoughts on this.

Yuchen Pei added a comment - 2024-07-16 03:40

Hi holyfoot, ptal thanks

df05e3115d8 upstream/bb-10.5-mdev-34589-trans-rollback-pass-error bb-10.5-ycp MDEV-34589 pass error from ha_rollback_trans() to trans_rollback_stmt()

Let me know if you think any other solution mentioned in my previous comment would be better.

Yuchen Pei added a comment - 2024-07-16 03:40 Hi holyfoot , ptal thanks df05e3115d8 upstream/bb-10.5-mdev-34589-trans-rollback-pass-error bb-10.5-ycp MDEV-34589 pass error from ha_rollback_trans() to trans_rollback_stmt() Let me know if you think any other solution mentioned in my previous comment would be better.

Sergei Golubchik added a comment - 2024-07-16 10:56

I don't see how rollback can fail. In particular, in your case rollback certainly does not fail, what fails is "before query" sql.

Arguably, it's not needed before the rollback, this would've been an easy fix. Alternatively, spider can simply close the connection if the "before query" sql fails on rollback — it'll cause an implicit rollback, and, again, it won't fail.
Or always call conn->thd->clear_error() in spider_db_mbase().
Still, I think the safest fix would be the first one, something like

--- a/storage/spider/spd_db_mysql.cc

+++ b/storage/spider/spd_db_mysql.cc

@@ -2612,13 +2612,15 @@ int spider_db_mbase::rollback(

   DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later);

   conn->mta_conn_mutex_lock_already = TRUE;

   conn->mta_conn_mutex_unlock_later = TRUE;

-  if (spider_db_query(

+  conn->in_before_query= TRUE;

+  error_num= spider_db_query(

     conn,

     SPIDER_SQL_ROLLBACK_STR,

     SPIDER_SQL_ROLLBACK_LEN,

-1,

-    need_mon)

-  ) {

+    need_mon);

+  conn->in_before_query= FALSE;

+  if (error_num) {

     is_error = conn->thd->is_error();

     error_num = spider_db_errorno(conn);

     if (

Sergei Golubchik added a comment - 2024-07-16 10:56 I don't see how rollback can fail. In particular, in your case rollback certainly does not fail, what fails is "before query" sql. Arguably, it's not needed before the rollback, this would've been an easy fix. Alternatively, spider can simply close the connection if the "before query" sql fails on rollback — it'll cause an implicit rollback, and, again, it won't fail. Or always call conn->thd->clear_error() in spider_db_mbase() . Still, I think the safest fix would be the first one, something like --- a/storage/spider/spd_db_mysql.cc +++ b/storage/spider/spd_db_mysql.cc @@ -2612,13 +2612,15 @@ int spider_db_mbase::rollback( DBUG_ASSERT(!conn->mta_conn_mutex_unlock_later); conn->mta_conn_mutex_lock_already = TRUE; conn->mta_conn_mutex_unlock_later = TRUE; - if (spider_db_query( + conn->in_before_query= TRUE; + error_num= spider_db_query( conn, SPIDER_SQL_ROLLBACK_STR, SPIDER_SQL_ROLLBACK_LEN, -1, - need_mon) - ) { + need_mon); + conn->in_before_query= FALSE; + if (error_num) { is_error = conn->thd->is_error(); error_num = spider_db_errorno(conn); if (

Yuchen Pei added a comment - 2024-07-18 09:37

Thanks for the suggestion. I've updated my commit now.

holyfoot, ptal this one instead:

63867f31b5d bb-10.5-mdev-34589 MDEV-34589 Do not execute before queries in spider_db_mbase::rollback()

Yuchen Pei added a comment - 2024-07-18 09:37 Thanks for the suggestion. I've updated my commit now. holyfoot , ptal this one instead: 63867f31b5d bb-10.5-mdev-34589 MDEV-34589 Do not execute before queries in spider_db_mbase::rollback()

Alexey Botchkov added a comment - 2024-09-25 21:55

ok to push.

Alexey Botchkov added a comment - 2024-09-25 21:55 ok to push.

Yuchen Pei added a comment - 2024-09-30 07:04

Thanks for the review - pusehd 282b92f0a2b461e99d6d8876cdc4a1673a8b6b62 to 10.5

Yuchen Pei added a comment - 2024-09-30 07:04 Thanks for the review - pusehd 282b92f0a2b461e99d6d8876cdc4a1673a8b6b62 to 10.5

People

Assignee:: Yuchen Pei

Reporter:: Roel Van de Paar

Votes:: 0 Vote for this issue

Watchers:: 4 Start watching this issue

Dates

Created:: 2024-07-14 23:19

Updated:: 2024-10-01 00:24

Resolved:: 2024-09-30 07:04

Git Integration

Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.

MariaDB Server