- developers - lists.mariadb.org

Re: [Maria-developers] 21a84e23cf3: MDEV-16437: merge 5.7 P_S replication instrumentation and tables
by Sergei Golubchik 09 Apr '21

09 Apr '21

Hi, Sujatha! Note, it's a review of the combined `git diff e5fc78 21a84e` On Mar 31, Sujatha wrote: > revision-id: 21a84e23cf3 (mariadb-10.5.2-307-g21a84e23cf3) > parent(s): 4e6de585ff7 > author: Sujatha <sujatha.sivakumar(a)mariadb.com> > committer: Sujatha <sujatha.sivakumar(a)mariadb.com> > timestamp: 2020-11-30 13:22:32 +0530 > message: > > MDEV-16437: merge 5.7 P_S replication instrumentation and tables That was pretty good, thanks! Just few small comments below, no big changes will be needed. > diff --git a/mysql-test/suite/multi_source/simple.result b/mysql-test/suite/multi_source/simple.result > index a66d49e88cb..b57e146feb5 100644 > --- a/mysql-test/suite/multi_source/simple.result > +++ b/mysql-test/suite/multi_source/simple.result > @@ -21,7 +21,21 @@ show all slaves status; > Connection_name Slave_SQL_State Slave_IO_State Master_Host Master_User Master_Port Connect_Retry Master_Log_File Read_Master_Log_Pos Relay_Log_File Relay_Log_Pos Relay_Master_Log_File Slave_IO_Running Slave_SQL_Running Replicate_Do_DB Replicate_Ignore_DB Replicate_Do_Table Replicate_Ignore_Table Replicate_Wild_Do_Table Replicate_Wild_Ignore_Table Last_Errno Last_Error Skip_Counter Exec_Master_Log_Pos Relay_Log_Space Until_Condition Until_Log_File Until_Log_Pos Master_SSL_Allowed Master_SSL_CA_File Master_SSL_CA_Path Master_SSL_Cert Master_SSL_Cipher Master_SSL_Key Seconds_Behind_Master Master_SSL_Verify_Server_Cert Last_IO_Errno Last_IO_Error Last_SQL_Errno Last_SQL_Error Replicate_Ignore_Server_Ids Master_Server_Id Master_SSL_Crl Master_SSL_Crlpath Using_Gtid Gtid_IO_Pos Replicate_Do_Domain_Ids Replicate_Ignore_Domain_Ids Parallel_Mode SQL_Delay SQL_Remaining_Delay Slave_SQL_Running_State Slave_DDL_Groups Slave_Non_Transactional_Groups Slave_Transactional_Groups Retried_transactions Max_relay_log_size Executed_log_entries Slave_received_heartbeats Slave_heartbeat_period Gtid_Slave_Pos > slave1 Slave has read all relay log; waiting for more updates Waiting for master to send event 127.0.0.1 root MYPORT_1 60 master-bin.000001 <read_master_log_pos> mysqld-relay-bin-slave1.000002 <relay_log_pos> master-bin.000001 Yes Yes 0 0 <read_master_log_pos> <relay_log_space1> None 0 No 0 No 0 0 1 No optimistic 0 NULL Slave has read all relay log; waiting for more updates 0 0 0 0 1073741824 7 0 60.000 > slave2 Slave has read all relay log; waiting for more updates Waiting for master to send event 127.0.0.1 root MYPORT_2 60 master-bin.000001 <read_master_log_pos> mysqld-relay-bin-slave2.000002 <relay_log_pos> master-bin.000001 Yes Yes 0 0 <read_master_log_pos> <relay_log_space1> None 0 No 0 No 0 0 2 No optimistic 0 NULL Slave has read all relay log; waiting for more updates 0 0 0 0 1073741824 7 0 60.000 > +# > +# MDEV:16437: merge 5.7 P_S replication instrumentation and tables > +# > +select * from performance_schema.replication_connection_configuration; > +CONNECTION_NAME HOST PORT USER USING_GTID SSL_ALLOWED SSL_CA_FILE SSL_CA_PATH SSL_CERTIFICATE SSL_CIPHER SSL_KEY SSL_VERIFY_SERVER_CERTIFICATE SSL_CRL_FILE SSL_CRL_PATH CONNECTION_RETRY_INTERVAL CONNECTION_RETRY_COUNT HEARTBEAT_INTERVAL IGNORE_SERVER_IDS REPL_DO_DOMAIN_IDS REPL_IGNORE_DOMAIN_IDS > +slave2 # # root NO NO NO 60 86400 60.000 > +slave1 # # root NO NO NO 60 86400 60.000 Isn't the host always 127.0.0.1 ? Why to mask it? Also, may be it'd be easier to read the result, if you print it vertically? It looks that CONNECTION_RETRY_COUNT is 86400. And 86400 is clearly a timeout, not a retry count. > start all slaves; > +# > +# MDEV:16437: merge 5.7 P_S replication instrumentation and tables > +# > +select * from performance_schema.replication_applier_status_by_coordinator; > +CONNECTION_NAME THREAD_ID SERVICE_STATE LAST_SEEN_TRANSACTION LAST_ERROR_NUMBER LAST_ERROR_MESSAGE LAST_ERROR_TIMESTAMP LAST_TRANS_RETRY_COUNT > +slave2 # ON 0 0000-00-00 00:00:00 0 > +slave1 # ON 0 0000-00-00 00:00:00 0 > stop slave 'slave1'; > show slave 'slave1' status; > Slave_IO_State > diff --git a/mysql-test/suite/rpl/include/rpl_deadlock.test b/mysql-test/suite/rpl/include/rpl_deadlock.test > index e9191d5fcd8..bccbe044a36 100644 > --- a/mysql-test/suite/rpl/include/rpl_deadlock.test > +++ b/mysql-test/suite/rpl/include/rpl_deadlock.test > @@ -59,6 +59,16 @@ let $status_var_comparsion= >; > connection slave; > SELECT COUNT(*) FROM t2; > COMMIT; > + > +--echo > +--echo # Test that the performance schema coulumn shows > 0 values. > +--echo > + > +--let $assert_text= current number of retries should be more than the value saved before deadlock. > +--let $assert_cond= [SELECT COUNT_TRANSACTIONS_RETRIES FROM performance_schema.replication_applier_status, COUNT_TRANSACTIONS_RETRIES, 1] > "$slave_retried_transactions" > +--source include/assert.inc what's wrong with simple SELECT COUNT_TRANSACTIONS_RETRIES > $slave_retried_transactions FROM performance_schema.replication_applier_status ? > + > +source include/check_slave_is_running.inc; > sync_with_master; > > # Check the data > diff --git a/mysql-test/suite/rpl/r/rpl_perfschema_connect_config.result b/mysql-test/suite/rpl/r/rpl_perfschema_connect_config.result > new file mode 100644 > index 00000000000..4ace84ffac4 > --- /dev/null > +++ b/mysql-test/suite/rpl/r/rpl_perfschema_connect_config.result > @@ -0,0 +1,124 @@ > +include/master-slave.inc > +[connection master] > +# Asserted this: On master, the table should return an empty set. > +connection slave; > + > +# Verify that SELECT works for every field and produces an output > +# similar to the corresponding field in SHOW SLAVE STATUS(SSS). > + > +include/assert.inc [Value returned by SSS and PS table for Host should be same.] > +include/assert.inc [Value returned by SSS and PS table for Port should be same.] > +include/assert.inc [Value returned by SSS and PS table for User should be same.] > +include/assert.inc [Value returned by SSS and PS table for Using_Gtid should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Allowed should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_CA_File should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_CA_Path should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Certificate should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Cipher should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Key should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Verify_Server_Certificate should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Crl_File should be same.] > +include/assert.inc [Value returned by SSS and PS table for SSL_Crl_Path should be same.] > +include/assert.inc [Value returned by SSS and PS table for Connection_Retry_Interval should be same.] > +include/assert.inc [Value returned by PS table for Connection_Retry_Count should be 10.] this is just unreadable > + > +# Heartbeat_Interval is part of I_S and P_S. We will compare the > +# two to make sure both match. ... > diff --git a/mysql-test/suite/rpl/t/rpl_perfschema_applier_config.test b/mysql-test/suite/rpl/t/rpl_perfschema_applier_config.test > new file mode 100644 > index 00000000000..132d9912222 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_perfschema_applier_config.test > @@ -0,0 +1,96 @@ > +# ==== Purpose ==== > +# > +# This test script serves as the functionality testing for the table > +# performance_schema.replication_applier_configuration. Test for ddl and dml > +# operations is a part of the perfschema suite. The ddl/dml tests are named: > +# 1) ddl_replication_applier_configuration.test and > +# 2) dml_replication_applier_configuration.test. > +# > +# The follwing scenarios are tested in this script: > +# > +# - Verify that output is same as SSS on a fresh slave. > +# - Verify that the value of this field is correct after STOP SLAVE. > +# - Verify that, when desired delay is set, the value is shown corectly. > +# - Verify that the value is preserved after STOP SLAVE. > +# - Verify that, when desired delay is reset, the value is shown corectly. > +# > +# ==== Related Worklog ==== > +# > +# MDEV-16437: merge 5.7 P_S replication instrumentation and tables > +# > + > +source include/master-slave.inc; > +source include/have_binlog_format_mixed.inc; master-slave should be included last > + > +let $assert_text= On master, the table should return an empty set.; > +let $assert_cond= count(*) = 0 from performance_schema.replication_applier_configuration; > +source include/assert.inc; again, what's wrong with just select * from performance_schema.replication_applier_configuration; or, may be, if you want to be very explicit select count(*) as 'must be 0' from performance_schema.replication_applier_configuration; > + > +--connection slave > + > +--echo > +--echo # Verify that SELECT works and produces an output similar to > +--echo # the corresponding field in SHOW SLAVE STATUS(SSS) in all scenarios. > +--echo > + > +--echo > +--echo # Verify that output is same as SSS on a fresh slave. > +--echo > + > +let $sss_value= query_get_value(SHOW SLAVE STATUS, SQL_Delay, 1); > +let $ps_value= query_get_value(select Desired_Delay from performance_schema.replication_applier_configuration, Desired_Delay, 1); > +let $assert_text= Value returned by SSS and PS table for Desired_Delay should be same.; > +let $assert_cond= "$sss_value" = "$ps_value"; > +source include/assert.inc; I'll stop commenting on every assert.inc, but please, please, stop overusing them. > + > +--echo > +--echo # Verify that the value of this field is correct after STOP SLAVE. > +--echo > + > +source include/stop_slave.inc; > + > +let $sss_value= query_get_value(SHOW SLAVE STATUS, SQL_Delay, 1); > +let $ps_value= query_get_value(select Desired_Delay from performance_schema.replication_applier_configuration, Desired_Delay, 1); > +let $assert_text= Value returned by SSS and PS table for Desired_Delay should be same.; > +let $assert_cond= "$sss_value" = "$ps_value"; > +source include/assert.inc; > + > +--echo > +--echo # Verify that, when desired delay is set, the value is shown corectly. > +--echo > + > +eval change master to master_delay= 2; > +source include/start_slave.inc; > + > +let $sss_value= query_get_value(SHOW SLAVE STATUS, SQL_Delay, 1); > +let $ps_value= query_get_value(select Desired_Delay from performance_schema.replication_applier_configuration, Desired_Delay, 1); > +let $assert_text= Value returned by SSS and PS table for Desired_Delay should be same.; > +let $assert_cond= "$sss_value" = "$ps_value"; > +source include/assert.inc; > + > +--echo > +--echo # Verify that the value is preserved after STOP SLAVE. > +--echo > + > +source include/stop_slave.inc; > + > +let $ss_value= query_get_value(SHOW SLAVE STATUS, SQL_Delay, 1); > +let $ps_value= query_get_value(select Desired_Delay from performance_schema.replication_applier_configuration, Desired_Delay, 1); > +let $assert_text= Value returned by SSS and PS table for Desired_Delay should be same.; > +let $assert_cond= "$sss_value" = "$ps_value"; > +source include/assert.inc; > + > +--echo > +--echo # Verify that, when desired delay is reset, the value is shown corectly. > +--echo > + > +eval change master to master_delay= 0; > +source include/start_slave.inc; > + > +let $sss_value= query_get_value(SHOW SLAVE STATUS, SQL_Delay, 1); > +let $ps_value= query_get_value(select Desired_Delay from performance_schema.replication_applier_configuration, Desired_Delay, 1); > +let $assert_text= Value returned by SSS and PS table for Desired_Delay should be same.; > +let $assert_cond= "$sss_value" = "$ps_value"; > +source include/assert.inc; > + > +source include/rpl_end.inc; > diff --git a/mysql-test/suite/rpl/t/rpl_perfschema_applier_status.test b/mysql-test/suite/rpl/t/rpl_perfschema_applier_status.test > new file mode 100644 > index 00000000000..52ee14cef2a > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_perfschema_applier_status.test > @@ -0,0 +1,72 @@ > +# ==== Purpose ==== > +# > +# This test script serves as the functionality testing for the table > +# performance_schema.replication_applier_status. Test for ddl and dml > +# operations is a part of the perfschema suite. The ddl/dml tests are named: > +# 1) ddl_replication_applier_status.test and > +# 2) dml_replication_applier_status.test. > +# > +# The follwing scenarios are tested in this script: > +# > +# - Verify that output is same as SSS on a fresh slave. > +# - Verify that the value of this field is correct after STOP SLAVE. > +# - Remaining delay is not tested. > +# - Count_trnsaction is partially tested here making sure it can be queried. > +# More testing in rpl/include/rpl_deadlock.test > +# > +# ==== Related Worklog ==== > +# > +# MDEV-16437: merge 5.7 P_S replication instrumentation and tables > +# > + > +source include/master-slave.inc; > +source include/have_binlog_format_mixed.inc; master-slave must always be last (I'll stop commenting on that too) > + > +let $assert_text= On master, the table should return an empty set.; > +let $assert_cond= count(*) = 0 from performance_schema.replication_applier_status; > +source include/assert.inc; > + > +--connection slave > + > +--echo > +--echo # Verify that SELECT works and produces an output similar to > +--echo # the corresponding field in SHOW SLAVE STATUS(SSS) in all scenarios. > +--echo > + > +--echo > +--echo # Verify that output is same as SSS on a fresh slave. > +--echo > + > +let $sss_value= query_get_value(SHOW SLAVE STATUS, Slave_SQL_Running, 1); > +let $ps_value= query_get_value(select Service_State from performance_schema.replication_applier_status, Service_State, 1); > +let $assert_text= SSS shows Slave_SQL_Running as "Yes". So, Service_State from this PS table should be "ON".; > +let $assert_cond= "$sss_value" = "Yes" AND "$ps_value"= "ON"; > +source include/assert.inc; > + > +let $ss_value= query_get_value(SHOW STATUS LIKE 'Slave_retried_transactions', Value, 1); > +let $ps_value= query_get_value(select count_transactions_retries from performance_schema.replication_applier_status, count_transactions_retries, 1); > +let $assert_text= COUNT_TRANSACTION_RETRIES should be equal to Slave_retried_transactions.; > +let $assert_cond= "$ps_value"= "$ss_value"; > +source include/assert.inc; > + > +--echo > +--echo # Verify that the fields show the correct values after STOP SLAVE. > +--echo > + > +source include/stop_slave.inc; > + > +let $sss_value= query_get_value(SHOW SLAVE STATUS, Slave_SQL_Running, 1); > +let $ps_value= query_get_value(select Service_State from performance_schema.replication_applier_status, Service_State, 1); > +let $assert_text= SSS shows Slave_SQL_Running as "No". So, Service_State from this PS table should be "OFF".; > +let $assert_cond= "$sss_value" = "No" AND "$ps_value"= "OFF"; > +source include/assert.inc; > + > +let $ss_value= query_get_value(SHOW STATUS LIKE 'Slave_retried_transactions', Value, 1); > +let $ps_value= query_get_value(select count_transactions_retries from performance_schema.replication_applier_status, count_transactions_retries, 1); > +let $assert_text= COUNT_TRANSACTION_RETRIES should be equal to Slave_retried_transactions.; > +let $assert_cond= "$ps_value"= "$ss_value"; > +source include/assert.inc; > + > +source include/start_slave.inc; > +source include/rpl_end.inc; > + > diff --git a/sql/rpl_mi.h b/sql/rpl_mi.h > index 4d47689ac18..946d138d618 100644 > --- a/sql/rpl_mi.h > +++ b/sql/rpl_mi.h > @@ -50,13 +57,6 @@ class Domain_id_filter > */ > DYNAMIC_ARRAY m_domain_ids[2]; > > -public: > - /* domain id list types */ > - enum enum_list_type { > - DO_DOMAIN_IDS= 0, > - IGNORE_DOMAIN_IDS > - }; > - Why did you move it up? Does it make any difference? > Domain_id_filter(); > > ~Domain_id_filter(); > diff --git a/storage/perfschema/table_replication_applier_status_by_coordinator.cc b/storage/perfschema/table_replication_applier_status_by_coordinator.cc > index beb8620b240..30cf56ce0c2 100644 > --- a/storage/perfschema/table_replication_applier_status_by_coordinator.cc > +++ b/storage/perfschema/table_replication_applier_status_by_coordinator.cc > @@ -55,12 +55,14 @@ table_replication_applier_status_by_coordinator::m_share= > sizeof(pos_t), /* ref length */ > &m_table_lock, > { C_STRING_WITH_LEN("CREATE TABLE replication_applier_status_by_coordinator(" > - "CHANNEL_NAME CHAR(64) collate utf8_general_ci not null," > + "CONNECTION_NAME VARCHAR(256) collate utf8_general_ci not null," please, don't rename existing columns, let's keep calling it CHANNEL_NAME everywhere. > "THREAD_ID BIGINT UNSIGNED," > "SERVICE_STATE ENUM('ON','OFF') not null," > + "LAST_SEEN_TRANSACTION CHAR(57) not null," I think it's better to put new columns at the end. > "LAST_ERROR_NUMBER INTEGER not null," > "LAST_ERROR_MESSAGE VARCHAR(1024) not null," > - "LAST_ERROR_TIMESTAMP TIMESTAMP(0) not null)") }, > + "LAST_ERROR_TIMESTAMP TIMESTAMP(0) not null," > + "LAST_TRANS_RETRY_COUNT INTEGER not null)") }, > false /* perpetual */ > }; > > @@ -104,15 +106,7 @@ int table_replication_applier_status_by_coordinator::rnd_next(void) > { > mi= (Master_info *)my_hash_element(&master_info_index->master_info_hash, m_pos.m_index); > > - /* > - Construct and display SQL Thread's (Coordinator) information in > - 'replication_applier_status_by_coordinator' table only in the case of > - multi threaded slave mode. Code should do nothing in the case of single > - threaded slave mode. In case of single threaded slave mode SQL Thread's > - status will be reported as part of > - 'replication_applier_status_by_worker' table. > - */ is this no longer true? > - if (mi && mi->host[0] && /*mi->rli.get_worker_count() > */ 0) > + if (mi && mi->host[0]) > { > make_row(mi); > m_next_pos.set_after(&m_pos); > @@ -147,13 +141,20 @@ int table_replication_applier_status_by_coordinator::rnd_pos(const void *pos) > void table_replication_applier_status_by_coordinator::make_row(Master_info *mi) > { > m_row_exists= false; > + rpl_gtid gtid; > + char buf[10+1+10+1+20+1]; > + String str(buf, sizeof(buf), system_charset_info); There's a special template for it: StringBuffer<10+1+10+1+20+1> str; > + bool first= true; > + > + str.length(0); not needed if you use StringBuffer<> > > DBUG_ASSERT(mi != NULL); > > mysql_mutex_lock(&mi->rli.data_lock); > > - m_row.channel_name_length= static_cast<uint>(mi->connection_name.length); > - memcpy(m_row.channel_name, mi->connection_name.str, m_row.channel_name_length); > + gtid= mi->rli.last_seen_gtid; > + m_row.connection_name_length= static_cast<uint>(mi->connection_name.length); > + memcpy(m_row.connection_name, mi->connection_name.str, m_row.connection_name_length); > > if (mi->rli.slave_running) > { > @@ -175,6 +176,18 @@ void table_replication_applier_status_by_coordinator::make_row(Master_info *mi) > else > m_row.service_state= PS_RPL_NO; > > + if ((gtid.seq_no > 0 && > + !rpl_slave_state_tostring_helper(&str, &gtid, &first))) > + { > + strmake(m_row.last_seen_transaction,str.ptr(), str.length()); > + m_row.last_seen_transaction_length= str.length(); > + } > + else > + { > + m_row.last_seen_transaction_length= 0; > + memcpy(m_row.last_seen_transaction, "", 1); a strange way to write m_row.last_seen_transaction[0]= 0, but ok :) > + } > + > mysql_mutex_lock(&mi->rli.err_lock); > > m_row.last_error_number= (long int) mi->rli.last_error().number; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 4

[Maria-developers] JSON_TABLE: Why one can't just use Name_resolution_context
by Sergey Petrunia 09 Apr '21

09 Apr '21

Hello, There was this question raised: Why can't JSON_TABLE code just use the current Name_resolution_context objects? == Context == Name_resolution_context object has these two members that specify a subset of tables to use: TABLE_LIST *first_name_resolution_table; TABLE_LIST *last_name_resolution_table; JSON_TABLE code also introduces this one: table_map ignored_tables; With the meaning that name resolution process will not search for the field of interest in the tables listed in the bitmap. == Rationale == For a JSON_TABLE invocation select ... from json_table(ARG, ...) as JT .... the ARG may refer to any table that precedes JT in the from clause. Let's call the set of the tables ALLOWED_DEPS. Can one take a JSON_TABLE invocation and pick first_name_resolution_table=FIRST, last_name_resolution_table=LAST such that ALLOWED_DEPS = { FIRST, SECOND= FIRST->next_name_resolution_table, THIRD= SECOND->next_name_resolution_table, ... LAST } ? I claim that there are cases where this is not possible. == An example == select * from t1, (t2 natural join (json_table('{}', '$' COLUMNS (d for ordinality)) as JT natural join t3 ) ); Here, JT's ALLOWED_DEPS={t1, t2}. Looking at the data structures (each Table name denotes a TABLE_LIST object referring to the table) select_lex.name_resolution_context.first_name_resolution_table= t1 t1->next_name_resolution_table= $join_nest1 $join_nest1->next_name_resolution_table= NULL $join_nest1->nested_join.join_list= { $join_nest2, t2 } t2->next_name_resolution_table= $join_nest2 $join_nest2->next_name_resolution_table= NULL $join_nest2->nested_join.join_list= {t3, JT} JT->next_name_resolution_table= t3 t3->next_name_resolution_table= NULL Apparently there is no single chain that includes t1 and t2. == Possible objections == "But nested (outer) joins do it" Nested outer joins never need to pick a subset that includes only - one of the tables in a NATURAL JOIN - one of the tables outside that NATURAL JOIN. so there's no comparison. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] ee538938345: MDEV-21117: refine the server binlog-based recovery for semisync
by Sergei Golubchik 08 Apr '21

08 Apr '21

Hi, Andrei! On Apr 07, Andrei Elkin wrote: > Sergei, salve. > > I should've asked for clarification to > > >>> + } > >>> + } else > >> > >> add into the else branch something like > >> > >> /* this branch is only for the legacy TC_LOG_MMAP */ > >> compile_time_assert(sizeof(TC_LOG_MMAP) > 1 ); > > > > [x] > > Actually the branch, which is > > } else > if (IF_WSREP((wsrep_emulate_bin_log && > wsrep_is_wsrep_xid(info->list + i) && > x <= wsrep_limit), false) || > (info->commit_list ? > my_hash_search(info->commit_list, (uchar *)&x, sizeof(x)) != 0 : > tc_heuristic_recover == TC_HEURISTIC_RECOVER_COMMIT)) > { > #ifndef DBUG_OFF > int rc= > #endif > hton->commit_by_xid(hton, info->list+i); > > deals with tc_heuristic_recover. > > I am not sure what you meant. I wanted to make sure all TC_LOG_MMAP code is removed when TC_LOG_MMAP itself goes away. But apparenlty I overlooked the heuristic recover path. So this code deals with both heuristic recover and TC_LOG_MMAP. Okay than, forget that comment. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] JSON_TABLE: on the question of temp table "cache" in MySQL-8
by Sergey Petrunia 07 Apr '21

07 Apr '21

1 0

Re: [Maria-developers] d221068ae8b: Change CHARSET_INFO character set and collaction names to LEX_CSTRING
by Sergei Golubchik 07 Apr '21

07 Apr '21

Hi, Michael! On Apr 01, Michael Widenius wrote: > revision-id: d221068ae8b (mariadb-10.5.2-540-gd221068ae8b) > parent(s): 896c3b0a00a > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-24 19:13:42 +0200 > message: > > Change CHARSET_INFO character set and collaction names to LEX_CSTRING > > This change removed 68 strlen() calls from the code. > > The following renames was to ensure we don't use the old names > when merging code from earlier releases, as using the new variables > for print function could result in crashes: > - charset->csname renamed to charset->cs_name > - charset->name renamed to charset->col_name Two comments: 1. please, use coll_name. in our codebase "cs" conventionally means "charset", "col" is usually used for "column", "coll" - for "collation" 2. You changed the client api, as can be seen from > diff --git a/client/mysql.cc b/client/mysql.cc > index e6c7d0f091c..8c64d01539d 100644 > --- a/client/mysql.cc > +++ b/client/mysql.cc > @@ -4719,9 +4719,8 @@ sql_real_connect(char *host,char *database,char *user,char *password, > return -1; // Retryable > } > > - charset_info= get_charset_by_name(mysql.charset->name, MYF(0)); > - > - > + charset_info= get_charset_by_name(IF_EMBEDDED(mysql.charset->col_name.str, > + mysql.charset->name), MYF(0)); > connected=1; > #ifndef EMBEDDED_LIBRARY > mysql_options(&mysql, MYSQL_OPT_RECONNECT, &debug_info_flag); Luckily, C/C is a separate project now, so most clients won't be affected. But anything linking with embedded might be. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] e3971006cca: Give a readable error in mtr if resolve_at_variable fails
by Sergei Golubchik 07 Apr '21

07 Apr '21

Hi, Monty! On Apr 05, Michael Widenius wrote: > revision-id: e3971006cca (mariadb-10.5.2-547-ge3971006cca) > parent(s): ae009435e72 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-24 19:25:09 +0200 > message: > > Give a readable error in mtr if resolve_at_variable fails This change is wrong. Clearly, the || '' part was intentional. Check the history, it was added in the commit commit 897b51db434 Author: Sergei Golubchik <serg(a)mariadb.com> Date: Thu Sep 10 12:12:47 2020 +0200 make S3 tests to run when S3 is statically linked * use the environment variable HA_S3_SO, not a literal ha_s3 in cnf files * make ConfigFactory to support empty option values * update no_s3.result after MDEV-11412 so, apparently, the idea was that you should be able to write plugin-load-add=$HA_S3_SO and it will *not* fail when $HA_S3_SO is not defined, because S3 engine is statically linked in. > diff --git a/mysql-test/lib/My/ConfigFactory.pm b/mysql-test/lib/My/ConfigFactory.pm > index 3249a06256c..5a5b691998c 100644 > --- a/mysql-test/lib/My/ConfigFactory.pm > +++ b/mysql-test/lib/My/ConfigFactory.pm > @@ -347,8 +347,15 @@ sub resolve_at_variable { > or croak "There is no group named '$group_name' that ", > "can be used to resolve '$option_name' for test '$self->{testname}'"; > > - my $value= $from_group->value($option_name) || ''; > - $res .= $before.$value; > + my $value= $from_group->value($option_name); > + if (!defined($value)) > + { > + ::mtr_verbose("group: $group_name option_name: $option_name is undefined"); > + } > + else > + { > + $res .= $before.$value; > + } > } > $res .= $after; > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 405a89353b2: Don't reset StringBuffers in loops when not needed
by Sergei Golubchik 06 Apr '21

06 Apr '21

Hi, Monty! On Apr 02, Michael Widenius wrote: > revision-id: 405a89353b2 (mariadb-10.5.2-542-g405a89353b2) > parent(s): 2dd8d472fcc > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-24 19:25:08 +0200 > message: > > Don't reset StringBuffers in loops when not needed > > - Moved out creating StringBuffers in loops and instead create them > outside and just reset the buffer if it was not allocated (to avoid > a possible malloc/free for every entry) > > diff --git a/sql/sql_string.h b/sql/sql_string.h > index 29d01779b71..8df50ac5e66 100644 > --- a/sql/sql_string.h > +++ b/sql/sql_string.h > @@ -1045,6 +1045,12 @@ class StringBuffer : public String > { > length(0); > } > + void set_buffer_if_not_allocated(CHARSET_INFO *cs) > + { > + if (is_alloced()) > + Static_binary_string::set(buff, buff_sz); it seems to be doing exactly the opposite of what the name suggests. It sets the buffer *if allocated*. Also, it looks logical to do length(0) here, so that this method would reset the StringBuffer to its initial state. > + set_charset(cs); > + } > }; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

[Maria-developers] MDEV-9504
by Jason Alpers 03 Apr '21

03 Apr '21

Hello, We're hitting the problem in MDEV-9504 ( https://jira.mariadb.org/browse/MDEV-9504) I see from the commit at https://github.com/MariaDB/server/commit/07b8aefe90ca830d2de068f2966cd2288b… that this change was backed out due to test failures. I ran through all the mysql-tests with and without the change and haven't been able to pinpoint a difference though. The reverting commit indicates the correct value for select_limit is only computed when the query plan changes. Any chance someone could provide more detail on what needs to be changed to avoid regression with this fix? If there are sample queries that regress with only the original MDEV-9504 changes, that would be helpful. Thanks in advance! Jase

1 0

Re: [Maria-developers] GSOC21: MDEV-16375 & MDEV-23143
by Hollow Man 03 Apr '21

03 Apr '21

Hi community! I've had my proposal shared with https://drive.google.com/file/d/1sv0qbqt9W-ob3GqxygWwRGurpRS1lCiv/view , hope to get some feedback from the community.   Songlin   ------------------ Original ------------------ From:  "Hollow Man"<hollowman(a)hollowman.ml>; Date:  Thu, Mar 11, 2021 00:17 AM To:  "maria-developers"<maria-developers(a)lists.launchpad.net>; Subject:  GSOC21: MDEV-16375 & MDEV-23143   Hi MariaDB community!    Glad to be here! My github account is @HollowMan6. Though I'm new to MariaDB community, I'm interested in MDEV-16375 & MDEV-23143: Function to normalize a json value & missing a JSON_EQUALS function for this year's GSOC project. Here are my first thoughts on these issues:    I have checked part of the codebase and I think the two issues can be merged into one. First we can create a function named JSON_NORMALIZE to normalize the json, which automatically parses the inputed json document, recursively sorts the keys (for objects) / sorts the numbers (for arrays), removes the spaces, and then return the json document string.    Then we create a function named JSON_EQUALS, which can be used to compare 2 json documents for equality realized by first seperately normalize the two json documents using JSON_NORMALIZE, then the 2 can be compared exactly as binary strings.    I have taken some inspirations from the Item_func_json_keys and json_scan_start for parsing json documents, and I think it's possible to sort the keys using std::map in STL for objects.    That's all for my ideas so far. Please correct me if I made some mistakes, and I'm going to work on my ideas later. Cheers! Hollow Man

3 4

Re: [Maria-developers] JSON_TABLE: On name resolution question
by Sergey Petrunia 02 Apr '21

02 Apr '21

On Thu, Apr 01, 2021 at 10:14:04PM -0700, Igor Babaev wrote: > Sergey, > > Can you explain this: > > MariaDB [test]> explain format=json select * from t1 as t2 where item_name > in (select item_name from t2 as s, json_table(t2.item_props,'$' columns( > color varchar(100) path '$.color')) as t) and t2.item_name='Jeans'; > +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > | EXPLAIN > > > > > > > | > +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > | { > "query_block": { > "select_id": 1, > "table": { > "table_name": "t2", > "access_type": "ALL", > "rows": 2, > "filtered": 100, > "attached_condition": "t2.item_name = 'Jeans'" > }, > "table": { > "table_name": "s", > "access_type": "ALL", > "rows": 2, > "filtered": 100, > "attached_condition": "s.item_name = 'Jeans'" > }, > "table": { > "table_name": "t", > "access_type": "ALL", > "rows": 40, > "filtered": 100, > "table_function": "json_table", > "first_match": "t2" > } > } > } | > +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > How do get get s.item_name = 'Jeans' ? > Check out the optimizer trace: First, the subquery is converted into a semi-join: "transformation": { "select_id": 2, "from": "IN (SELECT)", "to": "semijoin", "converted_to_semi_join": true } then, there's a multiple equality: "transformation": "equality_propagation", "resulting_condition": "1 and multiple equal('Jeans', t2.item_name, s.item_name)" and then, after substituion: "attaching_conditions_to_tables": { "original_condition": "t2.item_name = 'Jeans' and s.item_name = 'Jeans'", BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0