[MDEV-30895] Assertion `btr_cur->rtr_info->thr || !btr_cur->index()->is_committed()' failed in rtr_ins_enlarge_mbr after ALTER table force Created: 2023-03-21  Updated: 2023-04-28  Resolved: 2023-04-28

Status: Closed
Project: MariaDB Server
Component/s: GIS
Affects Version/s: 11.0
Fix Version/s: 11.0.1

Type: Bug Priority: Critical
Reporter: Alice Sherepa Assignee: Marko Mäkelä
Resolution: Fixed Votes: 0
Labels: not-10.11, regression

Attachments: File MDEV-30895.test    
Issue Links:
Problem/Incident
is caused by MDEV-29694 Remove the InnoDB change buffer Closed

 Description   

to repeat please run MDEV-30895.test

11.0

mariadbd: /11.0/src/storage/innobase/gis/gis0rtree.cc:1471: dberr_t rtr_ins_enlarge_mbr(btr_cur_t*, mtr_t*): Assertion `btr_cur->rtr_info->thr || !btr_cur->index()->is_committed()' failed.
230321 16:38:56 [ERROR] mysqld got signal 6 ;
 
 
Server version: 11.0.2-MariaDB-debug-log source revision: f6cb93ba8d24c2e67d396c3dd9ef2b0bb3a3e665
 
??:0(gsignal)[0x7f8c905f100b]
??:0(abort)[0x7f8c905d0859]
/lib/x86_64-linux-gnu/libc.so.6(+0x22729)[0x7f8c905d0729]
??:0(__assert_fail)[0x7f8c905e1fd6]
gis/gis0rtree.cc:1474(rtr_ins_enlarge_mbr(btr_cur_t*, mtr_t*))[0x55bda321afcb]
row/row0merge.cc:211(spatial_index_info::insert(unsigned long, btr_pcur_t*, bool&, mem_block_info_t*, mtr_t*))[0x55bda2de5339]
row/row0merge.cc:1754(row_merge_spatial_rows(unsigned long, spatial_index_info**, unsigned long, mem_block_info_t*, btr_pcur_t*, bool&, mtr_t*))[0x55bda2dc5ffe]
row/row0merge.cc:2143(row_merge_read_clustered_index(trx_t*, TABLE*, dict_table_t const*, dict_table_t*, bool, dict_index_t**, dict_index_t*, fts_psort_t*, merge_file_t*, unsigned long const*, unsigned long, dtuple_t const*, dict_add_v_col_t const*, unsigned long const*, unsigned long, ib_sequence_t&, unsigned char*, bool, pfs_os_file_t*, ut_stage_alter_t*, double, unsigned char*, TABLE*, bool, std::map<unsigned int, dict_col_t*, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, dict_corow/row0merge.cc:4793(row_merge_build_indexes(trx_t*, dict_table_t*, dict_table_t*, bool, dict_index_t**, unsigned long const*, unsigned long, TABLE*, dtuple_t const*, unsigned long const*, unsigned long, ib_sequence_t&, bool, ut_stage_alter_t*, dict_add_v_col_t const*, TABLE*, bool, std::map<unsigned int, dict_col_t*, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, dict_col_t*> > > const*))[0x55bda2ddc9f8]
handler/handler0alter.cc:8702(ha_innobase::inplace_alter_table(TABLE*, Alter_inplace_info*))[0x55bda2b13a17]
sql/handler.h:4824(handler::ha_inplace_alter_table(TABLE*, Alter_inplace_info*))[0x55bda1a98d0a]
sql/sql_table.cc:7744(mysql_inplace_alter_table(THD*, TABLE_LIST*, TABLE*, TABLE*, Alter_inplace_info*, MDL_request*, st_ddl_log_state*, TRIGGER_RENAME_PARAM*, Alter_table_ctx*, bool&, unsigned long long&, bool))[0x55bda1a72223]
sql/sql_table.cc:10846(mysql_alter_table(THD*, st_mysql_const_lex_string const*, st_mysql_const_lex_string const*, Table_specification_st*, TABLE_LIST*, Recreate_info*, Alter_info*, unsigned int, st_order*, bool, bool))[0x55bda1a8982b]
sql/sql_alter.cc:558(Sql_cmd_alter_table::execute(THD*))[0x55bda1c54bab]
sql/sql_parse.cc:6003(mysql_execute_command(THD*, bool))[0x55bda1789582]
sql/sql_parse.cc:7999(mysql_parse(THD*, char*, unsigned int, Parser_state*))[0x55bda1796848]
sql/sql_parse.cc:1896(dispatch_command(enum_server_command, THD*, char*, unsigned int, bool))[0x55bda176ca9e]
sql/sql_parse.cc:1407(do_command(THD*, bool))[0x55bda17697da]
sql/sql_connect.cc:1416(do_handle_one_connection(CONNECT*, bool))[0x55bda1c36c33]
sql/sql_connect.cc:1320(handle_one_connection)[0x55bda1c36590]
perfschema/pfs.cc:2203(pfs_spawn_thread)[0x55bda284bd2c]
nptl/pthread_create.c:478(start_thread)[0x7f8c90afc609]
 
Query (0x6290001092a8): ALTER  TABLE  t1 FORCE



 Comments   
Comment by Elena Stepanova [ 2023-03-25 ]

Another test case

--source include/have_innodb.inc
 
CREATE TABLE t (
  id INT AUTO_INCREMENT,
  c BINARY(226) DEFAULT '',
  s POINT NOT NULL,
  PRIMARY KEY(id,c),
  SPATIAL(s)
) ENGINE=InnoDB;
INSERT INTO t (s) VALUES
  (POINTFromText('POINT(0.78 0.72)')),(POINTFromText('POINT(0.44 0.21)')),
  (POINTFromText('POINT(0.93 0.56)')),(POINTFromText('POINT(0.57 0.21)')),
  (POINTFromText('POINT(0.12 0.65)')),(POINTFromText('POINT(0.20 0.96)')),
  (POINTFromText('POINT(0.99 0.71)')),(POINTFromText('POINT(0.27 0.23)')),
  (POINTFromText('POINT(0.68 0.14)')),(POINTFromText('POINT(0.20 0.05)')),
  (POINTFromText('POINT(0.47 0.57)')),(POINTFromText('POINT(0.89 0.79)')),
  (POINTFromText('POINT(0.09 0.57)')),(POINTFromText('POINT(0.58 0.52)')),
  (POINTFromText('POINT(0.73 0.32)')),(POINTFromText('POINT(0.87 0.35)')),
  (POINTFromText('POINT(0.60 0.12)')),(POINTFromText('POINT(0.14 0.17)')),
  (POINTFromText('POINT(0.76 0.29)')),(POINTFromText('POINT(0.60 0.35)')),
  (POINTFromText('POINT(0.48 0.69)')),(POINTFromText('POINT(0.79 0.45)')),
  (POINTFromText('POINT(0.85 0.11)')),(POINTFromText('POINT(0.59 0.99)')),
  (POINTFromText('POINT(0.95 0.18)')),(POINTFromText('POINT(0.78 0.49)')),
  (POINTFromText('POINT(0.11 0.22)')),(POINTFromText('POINT(0.26 0.85)')),
  (POINTFromText('POINT(0.28 0.10)')),(POINTFromText('POINT(0.45 0.25)')),
  (POINTFromText('POINT(0.70 0.40)')),(POINTFromText('POINT(0.65 0.86)')),
  (POINTFromText('POINT(0.69 0.98)')),(POINTFromText('POINT(0.56 0.11)')),
  (POINTFromText('POINT(0.94 0.59)')),(POINTFromText('POINT(0.19 0.94)')),
  (POINTFromText('POINT(0.82 0.85)')),(POINTFromText('POINT(0.74 0.07)')),
  (POINTFromText('POINT(0.33 0.48)')),(POINTFromText('POINT(0.37 0.37)')),
  (POINTFromText('POINT(0.40 0.08)')),(POINTFromText('POINT(0.45 0.74)')),
  (POINTFromText('POINT(0.57 0.07)')),(POINTFromText('POINT(0.36 0.11)')),
  (POINTFromText('POINT(0.94 0.60)')),(POINTFromText('POINT(0.75 0.76)')),
  (POINTFromText('POINT(0.92 0.56)')),(POINTFromText('POINT(0.88 0.52)')),
  (POINTFromText('POINT(0.49 0.24)')),(POINTFromText('POINT(0.96 0.08)')),
  (POINTFromText('POINT(0.93 0.99)')),(POINTFromText('POINT(0.88 0.31)')),
  (POINTFromText('POINT(0.93 0.78)')),(POINTFromText('POINT(0.62 0.50)')),
  (POINTFromText('POINT(0.54 0.53)')),(POINTFromText('POINT(0.66 0.83)')),
  (POINTFromText('POINT(0.21 0.87)')),(POINTFromText('POINT(0.42 0.28)')),
  (POINTFromText('POINT(0.80 0.84)')),(POINTFromText('POINT(0.39 0.68)')),
  (POINTFromText('POINT(0.05 0.24)')),(POINTFromText('POINT(0.05 0.58)'));
ALTER TABLE t FORCE;
 
# Cleanup
DROP TABLE t;

Comment by Elena Stepanova [ 2023-03-25 ]

The failure started happening after this commit in 11.0

commit f27e9c894779a4c7ebe6446ba9aa408f1771c114
Author: Marko Mäkelä
Date:   Wed Jan 11 17:59:36 2023 +0200
 
    MDEV-29694 Remove the InnoDB change buffer
    
    The purpose of the change buffer was to reduce random disk access,

Comment by Marko Mäkelä [ 2023-04-28 ]

The failing assertion was added in MDEV-29694 to catch any regression that would occur due to the following:

    btr_cur_t::thr: Remove. This field was originally needed for change
    buffering. Later, its use was extended to cover SPATIAL INDEX.
    Much of the time, rtr_info::thr holds this field. When it does not,
    we will add parameters to SPATIAL INDEX specific functions.

Before MDEV-29694, DML operations would have assigned the btr_cur_t::thr somewhere earlier.

I checked the original test on 10.6 (just to take some older branch), and I saw both !btr_cur->thr and !btr_cur->rtr_info->thr there as well.

I think that the relevant part of both MDEV-30895.test and elenst’s simpler test is that the table contains a SPATIAL INDEX and the assertion fails in a table rebuild operation. If I change the test to

CREATE TABLE t (
  id INT AUTO_INCREMENT,
  c BINARY(226) DEFAULT '',
  s POINT NOT NULL,
  PRIMARY KEY(id,c)
) ENGINE=InnoDB;
--…
ALTER TABLE t ADD SPATIAL INDEX(s);
ALTER TABLE t FORCE;

then the assertion will not fail on the first ALTER TABLE. With the following patch the test will pass for both ALTER TABLE:

diff --git a/storage/innobase/gis/gis0rtree.cc b/storage/innobase/gis/gis0rtree.cc
index 60218a132c9..f75eab07cf3 100644
--- a/storage/innobase/gis/gis0rtree.cc
+++ b/storage/innobase/gis/gis0rtree.cc
@@ -1468,7 +1468,8 @@ rtr_ins_enlarge_mbr(
 
 	/* Check path info is not empty. */
 	ut_ad(!btr_cur->rtr_info->parent_path->empty());
-	ut_ad(btr_cur->rtr_info->thr || !btr_cur->index()->is_committed());
+	ut_ad(btr_cur->rtr_info->thr || !btr_cur->index()->is_committed()
+	      || btr_cur->index()->table->name.is_temporary());
 
 	/* Create a memory heap. */
 	heap = mem_heap_create(1024);

The btr_cur->rtr_info->thr will only be needed in a call to rtr_page_get_father_block() for acquiring some R-tree locks. A thread that is executing DDL can safely pass the null pointer because it has exclusive access to the being-created index or the being-rebuilt copy of the table.

Generated at Thu Feb 08 10:19:42 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.