January 2020 - developers - lists.mariadb.org

Re: [Maria-developers] ef2519fee4e: MDEV-16546 System versioning setting to allow history modification
by Aleksey Midenkov 08 Jun '21

08 Jun '21

Hello, Sergei! On Fri, May 3, 2019 at 8:43 PM Sergei Golubchik <serg(a)mariadb.com> wrote: > > Hi, Aleksey! > > On May 03, Aleksey Midenkov wrote: > > revision-id: ef2519fee4e (versioning-1.0.5-17-gef2519fee4e) > > parent(s): 56145be2951 > > author: Aleksey Midenkov <midenok(a)gmail.com> > > committer: Aleksey Midenkov <midenok(a)gmail.com> > > timestamp: 2018-06-28 13:42:09 +0300 > > message: > > > > MDEV-16546 System versioning setting to allow history modification > > > > 1. Add server variable system_versioning_modify_history which will > > allow to set values for row_start, row_end in DML operations. > > > > 2. If secure_timestamp is YES or REPLICATION, > > system_versioning_modify_history does not have effect. If > > secure_timestamp is SUPER, system_versioning_modify_history requires > > special privilege (same as for setting current timestamp). > > I thought more about this idea. We don't really want to have the history > editable, do we? Well, I'm thinking about rollback table data to specific point in time. That could be a useful feature. > But it's needed for replication, to keep the master and > slave identical. That's what secure_timestamp is for. > > The idea was that this new variable, system_versioning_modify_history, > will be just a convenience feature, it will not allow history editing > any more than one can do without it. > > But now I suspect that even with secure_timestamp=NO one cannot truly > edit history. One can only insert new rows with arbitrary timestamps. > For example, to insert a row with row_start=1000 and row_end=2000, one > needs to do (if secure_timestamp=NO): > > set timestamp=1000; > insert row; > set timestamp=2000; > delete row; > > But I don't see how one can update or delete a history row with > secure_timestamp=NO. > > Now, with a SUPER privilege and secure_timestamp=NO or SUPER, one can > use the BINLOG command and truly edit the history arbitrarily, by faking > row events. I don't really get it why this is so important: since there is some limitation by configuration and privilege, we are just fine. Everything can be changed at filesystem level after all. > > The conclusion, I believe, is that system_versioning_modify_history > should allow INSERTs when secure_timestamp=NO, and it should allow > UPDATE/DELETE only for a SUPER user when secure_timestamp=NO or SUPER. I don't see a reason to argue on that. The only thing that is not clear, why we don't allow INSERTs when secure_timestamp=SUPER? > > The second thing I don't like at all, is when a table is created like > > CREATE TABLE t1 (a int) WITH SYSTEM VERSIONING > > with row_start/row_end implicit. You don't have it in the test, but > anyway one should be able to load history into such a table, while the > table does not have row_start and row_end columns. From the user point > of view these columns don't exist, they're pseudo-columns, like ROWID. > They just cannot be insertable-into, conceptually. But a user will want > to restore the history, right? I don't have a solution for this yet :( > Any ideas? We don't have to follow the conception if it doesn't help us. Since we have physical row_start/row_end, we don't have to pretend they don't exist. Who will win from that? > > See below a couple of minor comments about the patch itself. > > ... These are going to be fixed. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org -- All the best, Aleksey Midenkov @midenok

2 15

[Maria-developers] Lag Free ALTER Table for Slave
by Sachin Setiya 27 Feb '20

27 Feb '20

Hi Everyone! This mail will describe usage and implementation of Lag Free Alter. And this will also answer the question raised by Kristian and Simon J Mudd(in 2016) Desc:- This will split Alter into 2 different commits. START ALTER and COMMIT /ROLLBACK ALTER , Start Alter will be written in binlog as soon as we get the locks for the table, alter will proceeds as usual and at the time of writing binlog if alter is successful we will write COMMIT Alter other wise ROLLBACK Alter. Usage:- Using this feature is quite simple. 1. On master you have to turn on `BINLOG_SPLIT_ALTER` dynamic variable. 2. Slave must be using parallel replication. Advance Usage:- So alter is divided like this. 1. START identifier Actual_alter_stmt 2. COMMIT/ROLLBACK identifier Actual_alter_stmt. OR 2. COMMIT/ROLLBACK identifier ALTER identifier is thread_id. Questions by Simon Mudd. >>* this behaviour should be configurable? Yes. >> - new global variable on the master to allow injection of this changed event stream? Right , `BINLOG_SPLIT_ALTER` >> - a new session variable set in the session where the command is triggered ? Right , `BINLOG_SPLIT_ALTER` >> - on slave a setting to indicate how many INPLACE ALTER TABLEs can run at once? No setting so far , but I am thinking of maximum no of CONCURRENT ALTER = slave_parallel_threads >>* how does a DBA monitor what’s going on? >> - the progress and number of active DDL statements So as we there is 2 part, So progress will go like this 1. Executing of start alter (this will take most of time) 2. Waiting for commit/rollback Signal 3. Commit/ Rollback Alter. Number of active ALTER , these will create new threads so DBA can know using this or I am thinking of adding variable in SHOW SLAVE INFO. which will show active DDL. >> - please consider adding counters/metrics for: >> * number of “asynchronous DDLs” in progress / completed successfully / failed or rolled back Okay, We can have counter for these metrics. If i get time to implement this. >> * sizes of the DDL changes made so far Not sure if we need this. >> * number of threads busy in this state (maybe can be implied from SHOW PROCESSLIST or equivalent but nicer to have an easy to query metric) Show processlist will show the busy threads. Sample Output when concurrent alter is running on slave. slave_parallel_threads= 10 Every 1.0s: mysql -uroot -S /home/sachin/alter/build/mysql-test/var/tmp/mysqld.2.sock -e show processlist sachin-sp52: Wed Jan 22 16:56:00 2020 Id User Host db Command Time State Info Progress 1 system user NULL Daemon NULL InnoDB purge coordinator NULL 0.000 3 system user NULL Daemon NULL InnoDB purge worker NULL 0.000 2 system user NULL Daemon NULL InnoDB purge worker NULL 0.000 4 system user NULL Daemon NULL InnoDB purge worker NULL 0.000 5 system user NULL Daemon NULL InnoDB shutdown handler NULL 0.000 10 root localhost:44478 test Sleep 6 NULL 0.000 11 root localhost:44480 test Sleep 7 NULL 0.000 15 root localhost:44492 test Sleep 6 NULL 0.000 16 root localhost:44494 test Sleep 6 NULL 0.000 17 system user NULL Slave_IO 6 Waiting for master to send event NULL 0.000 19 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 22 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 20 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 23 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 24 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 25 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 26 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 21 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 27 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 28 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 18 system user NULL Slave_SQL 6 Slave has read all relay log; waiting for the slave I/O thread to update it NULL 0.000 29 system user test Slave_worker 6 NULL /*!100001 START 19 alter table t3 add column c int, force, algorithm=inplace */ 0.000 30 system user test Slave_worker 6 NULL /*!100001 START 17 alter table t1 add column c int, force, algorithm=inplace */ 0.000 31 system user test Slave_worker 6 NULL /*!100001 START 18 alter table t2 add column c int, force, algorithm=inplace */ 0.000 32 system user test Slave_worker 6 NULL /*!100001 START 20 alter table t4 add column c int, force, algorithm=inplace */ 0.000 33 system user test Slave_worker 6 NULL /*!100001 START 21 alter table t5 add column c int, force, algorithm=inplace */ 0.000 34 root localhost NULL Query 0 Init show processlist 0.000 >>* what happens if the commit/rollback ddl event never arrives? (the statement on the master could be skipped for one of several reasons) So here are 2 options If slave is still running DBA can manually run COMMIT/ROLLBACK identifier alter_stmt/ALTER , where identifier is same as START ALTER identifier. so this will either commit or rollback alter. If slave is not running. Since slave is not running all context is lost , so this will take as much as time a normal alter. >> - I assume that users on the slave see the old structure all the time until it completes Right, we work on copy of alter for myisam and for other case innodb will take care of it. >> - would this have a storage or performance penalty on the slave if the commit/rollback DDL event never arrives? Not performance but yes storage and ram penalty. >> - can a “commit” / “rollback” be manually triggered by the DBA in such circumstances? yes. >>* what happens if the server crashes while this process (or these processes) are ongoing when the server starts up again? So if we crash in START ALTER no issue , we can run it again , since it is transactional(we are working on copy so ..) If we crash after start alter and we receive COMMIT alter then it will be treated as normal alter. ROLLBACK ALTER will be instant. If we crash in COMMIT ALTER this will be same as crash in normal ALTER table , so DBA has to deal with it. Architecture:- Lets look at sample alter perf data for innodb and myisam Myisam + 96.72% 0.00% mysqld mysqld [.] mysql_parse + 96.72% 0.00% mysqld mysqld [.] mysql_execute_command + 96.72% 0.00% mysqld mysqld [.] Sql_cmd_alter_table::execute - 96.72% 0.00% mysqld mysqld [.] mysql_alter_table - mysql_alter_table - 96.63% copy_data_between_tables + 79.64% handler::ha_write_row + 6.24% TABLE::update_default_fields + 5.71% READ_RECORD::read_record + 1.87% do_copy_null 1.05% Field::do_field_int + 0.95% _mcount InnoDB + 41.27% 0.00% mysqld mysqld [.] mysql_parse + 41.27% 0.00% mysqld mysqld [.] mysql_execute_command + 41.27% 0.00% mysqld mysqld [.] Sql_cmd_alter_table::execute - 41.27% 0.00% mysqld mysqld [.] mysql_alter_table - mysql_alter_table - 41.27% mysql_inplace_alter_table - 41.15% handler::ha_inplace_alter_table ha_innobase::inplace_alter_table - row_merge_build_indexes - 41.00% row_merge_read_clustered_index + 22.88% row_merge_insert_index_tuples + 5.89% row_build_w_add_vcol + 5.20% BtrBulk::finish + 3.20% row_merge_buf_add + 0.87% page_cur_move_to_next 0.62% rec_get_offsets_func So as we can see most of the work is done by copy_data_between_tables/ mysql_inplace_alter_table. Master Side:- There is not much change into master side. So after locking table and before executing these functions we write START _id_ alter_stmt into binlog. And at the time of write_bin_log we write COMMIT _id_ alter_stmt into binlog. So alter statement will be divided into 2 , hence 2 gtid. START_ALTER will have special flag FL_START_ALTER_E1(4), this will be used on slave side to create new worker thread for start alter processing. Slave Side:- This require parallel replication on slave side. So in do_event when we get a gtid_event with FL_START_ALTER_E1, ::choose_thread will call rpl_parallel_add_extra_worker, which will create a new worker thread and this will do start alter processing. So the gtid_log_event and next Query_log_event will be scheduled to this new thread. And this thread will be exited as soon as alter is finished. On slave side START ALTER will binlog after successfully getting MDL lock and thd lock. Untill this point we are executed as DDL(so new GCO), but after getting locks we will call finish_event_group. So that new events can proceed in parallel. Then it will continue to execute code in mysql_alter_table until we reach a non transactional part(like renaming table/dropping table in myisam) So before executing NON-Transactional part it will wait signal from other worker thread to either abort or proceed forward. We will add a entry into mi-> start_alter_list, with thread_id(of master) as a key. COMMIT/ROLLBACK Alter is treated as a normal query_log_event so it will be assigned normal worker. This command will take SQLCOM_COMMIT_PREVIOUS path in mysql_execute_command. We will simple search for thread_id into start_alter_list and change status from ::WAITING to ::COMMIT_ALTER and signal the wait condition if we dont find the thread_id we will wait on wait condition, So simple it is just consumer producer between start alter and commit/rollback. Questions by Kristian:- >Can you explain what you mean by "true LOCK=NONE"? It is not clear from your >description. I think you mean something that will allow to run an ALTER >TABLE on the slave in parallel with the same statement on the master? Yes >There will be a number of things to consider with this. Previously, all >transactions in the binlog have been atomic; this introduces related events >that can be arbitrarily separated in the binlog. >For example, suppose the slave stops in the middle between BEGIN_DDL_EVENT >and COMMIT_DDL_EVENT/ROLLBACK_DDL_EVENT. What is then the slave's binlog >position / GTID position? Now we are using 2 gtid for ALTER , So it wont be issue. >Hopefully, the exiting pool of slave worker threads (currently used for >parallel replication) can be used for this as well? Doesn't seem right to >introduce a new kind of thread pool... We are using same(global) thread pool but I am creating new threads for START ALTER. Otherwise we will have a deadlock , Suppose if we have 5 concurrent START ALTER and just 5 slave-worker-threads , all worker will be waiting for COMMIT/ROLLBACK, but we cant execute commit rollback because we dont have any free worker. There can be more case when we have concurrent DML too , so creating a new thread was the safest option. >Won't you need some mechanism to prevent following events to access the >table before ALTER TABLE in a worker thread can acquire the metadata lock? Right, So Untill Locking START ALTER is executed as DDL so no following event will be executed before locking. And one more thing regarding locks , So there will we only one thread will be doing whole work of alter, (we can just assume that there is some sleep on slave) so execution on master and slave will be equivalent. >There are some nasty issues to be aware of with potential deadlocks related >to FLUSH TABLES WITH GLOBAL READ LOCK and such, when multiple threads are >trying to coordinate waiting and locking; there were already some quite hard >issues with this for parallel replication. I have to test for it. Code branch bb-10.5-olter Jira (Mdev-11675(https://jira.mariadb.org/browse/MDEV-11675)) Regards Sachin -- Regards Sachin Setiya Software Engineer at MariaDB

3 4

Re: [Maria-developers] bd98ef106c8: MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
by Sergei Golubchik 19 Feb '20

19 Feb '20

Hi, Aleksey! 1. It should be possible to enable/disable auto-creation. For example, CREATE TABLE ... PARTITION BY SYSTEM_TIME ... PARTITIONS AUTO; this solves few problems at once: * a user explicitly tells when auto-creation should work * you don't need to worry "if this name is already occupied" * you can define that these partitions can never overflow (for INTERVAL) * if you know that AUTO partitions never overflow, you can keep the old behavior for ALTER TABLE ADD PARTITION. 2. Separate thread is an interesting solution. Nicely avoids lots of problems. Still: * it's asynchronous, all tests have to take it into account * it's racy. "low probability" or not, with our number of users they'll hit it and rightfully will report it as a bug * if LOCK TABLE prevents it, partitions can overflow But I think these problems are easier to solve than those we'll face if auto-creation will happen in the main connection thread. So, let's keep your approach with a thread. But instead of going through the parser and mysql_execute_command, create a function that takes a TABLE or a TABLE_SHARE. And adds a partition there. It'll fix the "racy" part. This function can be called from a new thread. As for the LOCK TABLES - if you're under LOCK TABLES, you can simply call that function (to add a partition) directly at the end of the main INSERT/UPDATE/DELETE statement. It'll solve the last problem, LOCK TABLES won't prevent auto-creation. A couple of style comments: 1. create a one-liner make_partition_name (or something) with: sprintf(move_ptr, "p%u", i) and use it both in create_default_partition_names() and in your new code. Just to make sure partition names are always generated uniformly by the same function and if we'll want to change it to "pp%u" it should be done only in one place. 2. don't overload operators, use, say, bool greater_than(size_t seconds) But overall it's pretty good. A nice idea with a separate thread. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org On Nov 07, Aleksey Midenkov wrote: > revision-id: bd98ef106c8 (mariadb-10.4.7-55-gbd98ef106c8) > parent(s): 6d1fa01e8f1 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2019-08-19 12:01:14 +0300 > message: > > MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT > > When there are E empty partitions left, auto-create N new empty > partitions. > > This scheme must not allow partition overflow. I.e. E-fill time must > not exceed N-creation time. This means that low values for INTERVAL > and LIMIT must not be allowed for auto-creation. In case when overflow > is detected there is no need to do anything special: a warning will be > issued and the user will run manual rebuild to redistribute records > correctly. This is important because automatic ADD must be done fast, > without forced rebuild, by the obvious reason. > > Initial version implements hard-coded values of 1 for E and N. As well > as auto-creation threshold MinInterval = 1 hour, MinLimit = 1000. > > The name for newly added partition will be first chosen as "pX", where > X is partition number and "p" is hard-coded name prefix. If this name > is already occupied, the X will be incremented until the resulting > name will be free to use. > > Auto-creation mechanism is applied to every table having LIMIT or > INTERVAL clause. Note that there is no much sense in specifying > explicit partition list in this case and this is covered by > MDEV-19903. The syntax to explicitly turn it on/off as well as > user-defined values for E, N and name prefix is subject for further > discussion and feature requests. > > ALTER TABLE ADD PARTITION is now always fast. If there some history > partition overflow occurs manual ALTER TABLE REBUILD PARTITION is > needed.

2 5

[Maria-developers] [DISCUSS] Don't see ARM release information in MariaDB download page
by bo zhaobo 02 Feb '20

02 Feb '20

Hi Team, I want to make clear a thing about the Mariadb download page. Hope team could give some advices or help. As I had sent out an discussion to maria-docs, but didn't get some help. So I think I may get something in dev channel. When I want to download a aarch64 mariadb package, I'm failed to search any information in the content of page [1]. But I tried to search the RPM/DEB package, such as in '*Red Hat, Fedora, and CentOS Packages*' and '*Debian and Ubuntu Packages*', and I can see there are ARM(aarch64/arm64) specific packages in these linked pages [2]. And documentation in [1] just say OS/CPU with "*RedHat/CentOS/Fedora (x86, x86_64, ppc64, ppc64le)*". It seems that there is difference between the documentation with real released packages. That's why I'm here to want to make it clear and discuss with team. *How about add 'aarch64' in the OS/CPU description?* I think that would be great if community could fix that, because it is good for users. They could get what they want from MariaDB download page(MariaDB docs related). Thanks very much. BR ZhaoBo [1] https://downloads.mariadb.org/mariadb/10.5.0/ [2] http://ftp.igh.cnrs.fr/pub/mariadb//mariadb-10.5.0/yum/ http://ftp.igh.cnrs.fr/pub/mariadb//mariadb-10.5.0/repo/ [image: Mailtrack] <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=sig…> Sender notified by Mailtrack <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=sig…> 20/01/06 下午07:31:54

3 7

778e96749bc: MDEV-20923:UBSAN: member access within address … which does not point to an object of type 'xid_count_per_binlog'
by sujatha 23 Jan '20

23 Jan '20

revision-id: 778e96749bcadc5528bb7f214a711272f7413b96 (mariadb-10.1.43-66-g778e96749bc) parent(s): 982294ac1680938ac9223fb64a64e21f0cbc322a author: Sujatha committer: Sujatha timestamp: 2020-01-23 16:17:55 +0530 message: MDEV-20923:UBSAN: member access within address … which does not point to an object of type 'xid_count_per_binlog' Problem: ------- Accessing a member within 'xid_count_per_binlog' structure results in following error when 'UBSAN' is enabled. member access within address 0xXXX which does not point to an object of type 'xid_count_per_binlog' Analysis: --------- The problem appears to be that no constructor for 'xid_count_per_binlog' is being called, and thus the vtable will not be initialized. Fix: --- Defined a parameterized constructor for 'xid_count_per_binlog' class. --- sql/log.cc | 27 +++++++++++---------------- sql/log.h | 13 ++++++++++++- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/sql/log.cc b/sql/log.cc index acf1f8f8a9c..0efef6d1e29 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -3216,7 +3216,7 @@ void MYSQL_BIN_LOG::cleanup() DBUG_ASSERT(!binlog_xid_count_list.head()); WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::cleanup(): Removing xid_list_entry " "for %s (%lu)", b); - my_free(b); + delete b; } mysql_mutex_destroy(&LOCK_log); @@ -3580,17 +3580,9 @@ bool MYSQL_BIN_LOG::open(const char *log_name, */ uint off= dirname_length(log_file_name); uint len= strlen(log_file_name) - off; - char *entry_mem, *name_mem; - if (!(new_xid_list_entry = (xid_count_per_binlog *) - my_multi_malloc(MYF(MY_WME), - &entry_mem, sizeof(xid_count_per_binlog), - &name_mem, len, - NULL))) + new_xid_list_entry= new xid_count_per_binlog(log_file_name+off, (int)len); + if (!new_xid_list_entry) goto err; - memcpy(name_mem, log_file_name+off, len); - new_xid_list_entry->binlog_name= name_mem; - new_xid_list_entry->binlog_name_len= len; - new_xid_list_entry->xid_count= 0; /* Find the name for the Initial binlog checkpoint. @@ -3607,7 +3599,10 @@ bool MYSQL_BIN_LOG::open(const char *log_name, mysql_mutex_unlock(&LOCK_xid_list); if (!b) b= new_xid_list_entry; - strmake(buf, b->binlog_name, b->binlog_name_len); + if (b->binlog_name) + strmake(buf, b->binlog_name, b->binlog_name_len); + else + goto err; Binlog_checkpoint_log_event ev(buf, len); DBUG_EXECUTE_IF("crash_before_write_checkpoint_event", flush_io_cache(&log_file); @@ -3711,7 +3706,7 @@ bool MYSQL_BIN_LOG::open(const char *log_name, { WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::open(): Removing xid_list_entry for " "%s (%lu)", b); - my_free(binlog_xid_count_list.get()); + delete binlog_xid_count_list.get(); } mysql_cond_broadcast(&COND_xid_list); WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::open(): Adding new xid_list_entry for " @@ -3758,7 +3753,7 @@ Turning logging off for the whole duration of the MySQL server process. \ To turn it on again: fix the cause, \ shutdown the MySQL server and restart it.", name, errno); if (new_xid_list_entry) - my_free(new_xid_list_entry); + delete new_xid_list_entry; if (file >= 0) mysql_file_close(file, MYF(0)); close(LOG_CLOSE_INDEX); @@ -4252,7 +4247,7 @@ bool MYSQL_BIN_LOG::reset_logs(THD *thd, bool create_new_log, DBUG_ASSERT(b->xid_count == 0); WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::reset_logs(): Removing " "xid_list_entry for %s (%lu)", b); - my_free(binlog_xid_count_list.get()); + delete binlog_xid_count_list.get(); } mysql_cond_broadcast(&COND_xid_list); reset_master_pending--; @@ -9736,7 +9731,7 @@ TC_LOG_BINLOG::mark_xid_done(ulong binlog_id, bool write_checkpoint) break; WSREP_XID_LIST_ENTRY("TC_LOG_BINLOG::mark_xid_done(): Removing " "xid_list_entry for %s (%lu)", b); - my_free(binlog_xid_count_list.get()); + delete binlog_xid_count_list.get(); } mysql_mutex_unlock(&LOCK_xid_list); diff --git a/sql/log.h b/sql/log.h index b4c9b24a3a9..277e5c6f69c 100644 --- a/sql/log.h +++ b/sql/log.h @@ -587,7 +587,18 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG long xid_count; /* For linking in requests to the binlog background thread. */ xid_count_per_binlog *next_in_queue; - xid_count_per_binlog(); /* Give link error if constructor used. */ + xid_count_per_binlog(char *log_file_name, uint log_file_name_len) + :binlog_id(0), xid_count(0) + { + binlog_name_len= log_file_name_len; + binlog_name= (char *) my_malloc(binlog_name_len, MYF(MY_ZEROFILL)); + if (binlog_name) + memcpy(binlog_name, log_file_name, binlog_name_len); + } + ~xid_count_per_binlog() + { + my_free(binlog_name); + } }; I_List<xid_count_per_binlog> binlog_xid_count_list; mysql_mutex_t LOCK_binlog_background_thread;

1 0

d9716cfadb8: MDEV-20923:UBSAN: member access within address … which does not point to an object of type 'xid_count_per_binlog'
by sujatha 23 Jan '20

23 Jan '20

revision-id: d9716cfadb8cd3448f25d8b411451f87064cecaf (mariadb-10.1.43-66-gd9716cfadb8) parent(s): 982294ac1680938ac9223fb64a64e21f0cbc322a author: Sujatha committer: Sujatha timestamp: 2020-01-23 16:00:41 +0530 message: MDEV-20923:UBSAN: member access within address … which does not point to an object of type 'xid_count_per_binlog' Problem: ------- Accessing a member within 'xid_count_per_binlog' structure results in following error when 'UBSAN' is enabled. member access within address 0xXXX which does not point to an object of type 'xid_count_per_binlog' Analysis: --------- The problem appears to be that no constructor for 'xid_count_per_binlog' is being called, and thus the vtable will not be initialized. Fix: --- Defined a parameterized constructor for 'xid_count_per_binlog' class. --- sql/log.cc | 27 +++++++++++---------------- sql/log.h | 14 +++++++++++++- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/sql/log.cc b/sql/log.cc index acf1f8f8a9c..0efef6d1e29 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -3216,7 +3216,7 @@ void MYSQL_BIN_LOG::cleanup() DBUG_ASSERT(!binlog_xid_count_list.head()); WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::cleanup(): Removing xid_list_entry " "for %s (%lu)", b); - my_free(b); + delete b; } mysql_mutex_destroy(&LOCK_log); @@ -3580,17 +3580,9 @@ bool MYSQL_BIN_LOG::open(const char *log_name, */ uint off= dirname_length(log_file_name); uint len= strlen(log_file_name) - off; - char *entry_mem, *name_mem; - if (!(new_xid_list_entry = (xid_count_per_binlog *) - my_multi_malloc(MYF(MY_WME), - &entry_mem, sizeof(xid_count_per_binlog), - &name_mem, len, - NULL))) + new_xid_list_entry= new xid_count_per_binlog(log_file_name+off, (int)len); + if (!new_xid_list_entry) goto err; - memcpy(name_mem, log_file_name+off, len); - new_xid_list_entry->binlog_name= name_mem; - new_xid_list_entry->binlog_name_len= len; - new_xid_list_entry->xid_count= 0; /* Find the name for the Initial binlog checkpoint. @@ -3607,7 +3599,10 @@ bool MYSQL_BIN_LOG::open(const char *log_name, mysql_mutex_unlock(&LOCK_xid_list); if (!b) b= new_xid_list_entry; - strmake(buf, b->binlog_name, b->binlog_name_len); + if (b->binlog_name) + strmake(buf, b->binlog_name, b->binlog_name_len); + else + goto err; Binlog_checkpoint_log_event ev(buf, len); DBUG_EXECUTE_IF("crash_before_write_checkpoint_event", flush_io_cache(&log_file); @@ -3711,7 +3706,7 @@ bool MYSQL_BIN_LOG::open(const char *log_name, { WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::open(): Removing xid_list_entry for " "%s (%lu)", b); - my_free(binlog_xid_count_list.get()); + delete binlog_xid_count_list.get(); } mysql_cond_broadcast(&COND_xid_list); WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::open(): Adding new xid_list_entry for " @@ -3758,7 +3753,7 @@ Turning logging off for the whole duration of the MySQL server process. \ To turn it on again: fix the cause, \ shutdown the MySQL server and restart it.", name, errno); if (new_xid_list_entry) - my_free(new_xid_list_entry); + delete new_xid_list_entry; if (file >= 0) mysql_file_close(file, MYF(0)); close(LOG_CLOSE_INDEX); @@ -4252,7 +4247,7 @@ bool MYSQL_BIN_LOG::reset_logs(THD *thd, bool create_new_log, DBUG_ASSERT(b->xid_count == 0); WSREP_XID_LIST_ENTRY("MYSQL_BIN_LOG::reset_logs(): Removing " "xid_list_entry for %s (%lu)", b); - my_free(binlog_xid_count_list.get()); + delete binlog_xid_count_list.get(); } mysql_cond_broadcast(&COND_xid_list); reset_master_pending--; @@ -9736,7 +9731,7 @@ TC_LOG_BINLOG::mark_xid_done(ulong binlog_id, bool write_checkpoint) break; WSREP_XID_LIST_ENTRY("TC_LOG_BINLOG::mark_xid_done(): Removing " "xid_list_entry for %s (%lu)", b); - my_free(binlog_xid_count_list.get()); + delete binlog_xid_count_list.get(); } mysql_mutex_unlock(&LOCK_xid_list); diff --git a/sql/log.h b/sql/log.h index b4c9b24a3a9..e69c9c39eaf 100644 --- a/sql/log.h +++ b/sql/log.h @@ -587,7 +587,19 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG long xid_count; /* For linking in requests to the binlog background thread. */ xid_count_per_binlog *next_in_queue; - xid_count_per_binlog(); /* Give link error if constructor used. */ + xid_count_per_binlog(char *log_file_name, uint log_file_name_len) + :binlog_name(log_file_name), binlog_name_len(log_file_name_len), + binlog_id(0), xid_count(0) + { + binlog_name_len= log_file_name_len; + binlog_name= (char *) my_malloc(binlog_name_len, MYF(MY_ZEROFILL)); + if (binlog_name) + memcpy(binlog_name, log_file_name, binlog_name_len); + } + ~xid_count_per_binlog() + { + my_free(binlog_name); + } }; I_List<xid_count_per_binlog> binlog_xid_count_list; mysql_mutex_t LOCK_binlog_background_thread;

1 0

[Maria-developers] 43f27ae4778: MDEV-21490: binlog tests fail with valgrind: Conditional jump or move depends on uninitialised value in sql_ex_info::init
by sujatha 23 Jan '20

23 Jan '20

revision-id: 43f27ae4778b3e7be86897e49c182dc05080fd88 (mariadb-10.1.43-66-g43f27ae4778) parent(s): 982294ac1680938ac9223fb64a64e21f0cbc322a author: Sujatha committer: Sujatha timestamp: 2020-01-23 11:57:46 +0530 message: MDEV-21490: binlog tests fail with valgrind: Conditional jump or move depends on uninitialised value in sql_ex_info::init Problem: ======= P1) Conditional jump or move depends on uninitialised value(s) sql_ex_info::init(char const*, char const*, bool) (log_event.cc:3083) code: All the following variables are not initialized. ---- return ((cached_new_format != -1) ? cached_new_format : (cached_new_format=(field_term_len > 1 || enclosed_len > 1 || line_term_len > 1 || line_start_len > 1 || escaped_len > 1))); P2) Conditional jump or move depends on uninitialised value(s) Rows_log_event::Rows_log_event(char const*, unsigned int, Format_description_log_event const*) (log_event.cc:9571) Code: Uninitialized values is reported for 'var_header_len' variable. ---- if (var_header_len < 2 || event_len < static_cast<unsigned int>(var_header_len + (post_start - buf))) P3) Conditional jump or move depends on uninitialised value(s) Table_map_log_event::pack_info(Protocol*) (log_event.cc:11553) code:'m_table_id' is uninitialized. ---- void Table_map_log_event::pack_info(Protocol *protocol) ... size_t bytes= my_snprintf(buf, sizeof(buf), "table_id: %lu (%s.%s)", m_table_id, m_dbnam, m_tblnam); Fix: === P1 - Fix) Initialize cached_new_format,field_term_len, enclosed_len, line_term_len, line_start_len, escaped_len members in default constructor. P2 - Fix) "var_header_len" is initialized by reading the event buffer. In case of an invalid event the buffer will contain invalid data. Hence added a check to validate the event data. If event_len is smaller than valid header length return immediately. P3 - Fix) 'm_table_id' within Table_map_log_event is initialized by reading data from the event buffer. Use 'VALIDATE_BYTES_READ' macro to validate the current state of the buffer. If it is invalid return immediately. --- sql/log_event.cc | 7 +++++++ sql/log_event.h | 9 ++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/sql/log_event.cc b/sql/log_event.cc index e8881c77f2b..9924b9a0493 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -9535,6 +9535,12 @@ Rows_log_event::Rows_log_event(const char *buf, uint event_len, uint8 const post_header_len= description_event->post_header_len[event_type-1]; + if (event_len < (uint)(common_header_len + post_header_len)) + { + m_cols.bitmap= 0; + DBUG_VOID_RETURN; + } + DBUG_PRINT("enter",("event_len: %u common_header_len: %d " "post_header_len: %d", event_len, common_header_len, @@ -11043,6 +11049,7 @@ Table_map_log_event::Table_map_log_event(const char *buf, uint event_len, const char *post_start= buf + common_header_len; post_start+= TM_MAPID_OFFSET; + VALIDATE_BYTES_READ(post_start, buf, event_len); if (post_header_len == 6) { /* Master is of an intermediate source tree before 5.1.4. Id is 4 bytes */ diff --git a/sql/log_event.h b/sql/log_event.h index 2c8dc3d7353..04fce65faeb 100644 --- a/sql/log_event.h +++ b/sql/log_event.h @@ -2057,7 +2057,14 @@ class Query_log_event: public Log_event ****************************************************************************/ struct sql_ex_info { - sql_ex_info() {} /* Remove gcc warning */ + sql_ex_info(): + cached_new_format(-1), + field_term_len(0), + enclosed_len(0), + line_term_len(0), + line_start_len(0), + escaped_len(0) + {} /* Remove gcc warning */ const char* field_term; const char* enclosed; const char* line_term;

1 0

[Maria-developers] A cleanup for libmariadb
by Alexander Barkov 20 Jan '20

20 Jan '20

Hi Georg, Can you please review this cleanup patch? It's a prerequisite for: MDEV-17832 Protocol: extensions for Pluggable types and JSON, GEOMETRY The idea is that this patch introduces a new file libmariadb/mariadb_priv.h and adds prototypes of these functions: void free_rows(MYSQL_DATA *cur); int ma_multi_command(MYSQL *mysql, enum enum_multi_status status); MYSQL_FIELD * unpack_fields(MYSQL_DATA *data, MA_MEM_ROOT *alloc,uint fields, my_bool default_value); and removes these prototypes from mariadb_stmt.c. I think it should be safer this way. Later I'll add there more prototypes into libmariadb/mariadb_priv.h, e.g. functions that will be added under terms of MDEV-17832. Note, I also removed the "my_bool long_flag_protocol" parameter from unpack_fields(), it's not used anyway. Under terms of MDEV-17832 I'll have to add a "const MYSQL *mysql" parameter. So if we ever need to check CLIENT_LONG_FLAG inside unpack_fields() again, it can be done through the "mysql" parameter: (mysql->server_capabilities & CLIENT_LONG_FLAG) Thanks!

1 0

[Maria-developers] A cleanup for libmariadb
by Alexander Barkov 20 Jan '20

20 Jan '20

Hi Georg, Can you please review this cleanup patch? It's a prerequisite for: MDEV-17832 Protocol: extensions for Pluggable types and JSON, GEOMETRY The idea is that this patch introduces a new file libmariadb/mariadb_priv.h and adds prototypes of these functions: void free_rows(MYSQL_DATA *cur); int ma_multi_command(MYSQL *mysql, enum enum_multi_status status); MYSQL_FIELD * unpack_fields(MYSQL_DATA *data, MA_MEM_ROOT *alloc,uint fields, my_bool default_value); and removes these prototypes from mariadb_stmt.c. I think it should be safer this way. Later I'll add there more prototypes into libmariadb/mariadb_priv.h, e.g. functions that will be added under terms of MDEV-17832. Note, I also removed the "my_bool long_flag_protocol" parameter from unpack_fields(), it's not used anyway. Under terms of MDEV-17832 I'll have to add a "const MYSQL *mysql" parameter. So if we ever need to check CLIENT_LONG_FLAG inside unpack_fields() again, it can be done through the "mysql" parameter: (mysql->server_capabilities & CLIENT_LONG_FLAG) Thanks!

1 0

Re: [Maria-developers] 9d2c63d45e4: MDEV-20076: SHOW GRANTS does not quote role names properly
by Sergei Golubchik 19 Jan '20

19 Jan '20

Hi, Oleksandr! On Jan 19, Oleksandr Byelkin wrote: > revision-id: 9d2c63d45e4 (mariadb-10.3.20-6-g9d2c63d45e4) > parent(s): d4edb0510ec > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2019-11-14 09:32:54 +0100 > message: > > MDEV-20076: SHOW GRANTS does not quote role names properly > > Quotes added to output. > > diff --git a/mysql-test/main/grant5.test b/mysql-test/main/grant5.test > index 649bba7d1ca..045cbf8fc86 100644 > --- a/mysql-test/main/grant5.test > +++ b/mysql-test/main/grant5.test > @@ -33,3 +33,26 @@ REVOKE EXECUTE ON PROCEDURE sp FROM u; > --error ER_TABLE_NOT_LOCKED > REVOKE PROCESS ON *.* FROM u; > DROP TABLE t1; > + > +--echo # > +--echo # MDEV-20076: SHOW GRANTS does not quote role names properly > +--echo # > + > +create role 'role-1'; > +create role 'rock\'n\'roll'; > +create user 'user-1'@'localhost'; > +create user 'O\'Brien'@'localhost'; > +grant select on mysql.user to 'role-1'; > +grant select on mysql.user to 'rock\'n\'roll'; > +GRANT 'role-1' TO 'user-1'@'localhost'; > +GRANT 'rock\'n\'roll' TO 'O\'Brien'@'localhost'; > +show grants for 'role-1'; > +show grants for 'rock\'n\'roll'; > +show grants for 'user-1'@'localhost'; > +show grants for 'O\'Brien'@'localhost'; > +drop role 'role-1'; > +drop role 'rock\'n\'roll'; > +drop user 'user-1'@'localhost'; > +drop user 'O\'Brien'@'localhost'; > + > +--echo # End of 10.3 tests please add few tests when sql_quote_show_create=0. Like 1. sql_quote_show_create=0, role name is not reserved (e.g. foo) 2. sql_quote_show_create=0, role name is reserved (e.g. fetch) 3. sql_quote_show_create=0, role name is not a valid identifier (e.g. role-1) Then ok to push. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0