July 2019 - 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] Please review MDEV-18319 BIGINT UNSIGNED Performance issue
by Alexander Barkov 29 Oct '19

29 Oct '19

Hi Sergei, Please review a patch for MDEV-18319 https://github.com/MariaDB/server/commit/8a990ad17746927c6d395ec755a262eda5… Thanks

2 1

[Maria-developers] Weekly Report
by Rucha Deodhar 29 Jul '19

29 Jul '19

Report for week 9: Hello! This week I worked on documentation, added some comments to the code and removed some lines of code from parser as it was still making use of SELECT_LEX::item_list after opt_select_expressions. I was referring DELETE...RETURNING test case and noticed case for using stored functions was missing. I added that to INSERT...RETURNING and REPLACE...RETURNING. Also checked if the implementation supports other functions like string functions, date-time functions, control flow functions, numeric function and secondary functions(Since the aggregate functions cannot be used, I wanted to check if these functions can be used) I have made related changes in the documentation and my github repo is up to date with latest changes. Regards, Rucha Deodhar

1 0

[Maria-developers] Windows line endings in MariaDB sources again (
by Otto Kekäläinen 24 Jul '19

24 Jul '19

Hello! While trying to release 10.0.26 for Debian and Ubuntu I noticed git chokes again on Windows line endings. As our .gitattributes states that by default git should clean up the line endings, why do we keep getting new Windows line endings in new code? mariadb-10.0$ cat .gitattributes # Set the default behavior, in case people don't have core.autocrlf set. * text=auto ... storage/connect/mysql-test/connect/std_data/*.txt -text mariadb-10.0.26$ find . | xargs file | grep CRLF ./storage/connect/mysql-test/connect/std_data/boyswin.txt: ASCII text, with CRLF line terminators ./storage/connect/mysql-test/connect/std_data/expenses.txt: ASCII text, with CRLF line terminators ./storage/connect/mysql-test/connect/std_data/emp.txt: ASCII text, with CRLF line terminators ./mysql-test/std_data/loaddata7.dat: ASCII text, with CRLF line terminators ./mysql-test/r/loadxml.result: ASCII text, with CRLF, LF line terminators ./mysql-test/r/perror-win.result: ASCII text, with CRLF, LF line terminators ./mysql-test/r/mysql_binary_mode.result: ASCII English text, with CRLF, LF line terminators ./mysql-test/r/func_regexp_pcre.result: UTF-8 Unicode C++ program text, with CRLF, CR, LF line terminators ./pcre/testdata/grepoutputN: ASCII text, with CRLF, CR, LF line terminators ./pcre/testdata/greppatN4: ASCII text, with CRLF line terminators This leads for me to: dpkg-source: info: building mariadb-10.0 using existing ./mariadb-10.0_10.0.26.orig.tar.gz dpkg-source: info: local changes detected, the modified files are: mariadb-10.0/storage/connect/mysql-test/connect/std_data/boyswin.txt mariadb-10.0/storage/connect/mysql-test/connect/std_data/emp.txt mariadb-10.0/storage/connect/mysql-test/connect/std_data/expenses.txt dpkg-source: error: aborting due to unexpected upstream changes, see /tmp/mariadb-10.0_10.0.26-1.diff.Yzwwhw Which means I have to do extra work with importing the upstream sources.. - Otto

2 4

[Maria-developers] Weekly Report
by Rucha Deodhar 22 Jul '19

22 Jul '19

Report for week 8: Hello! This week I worked on the parser. I removed the hack "swapping lists" we had used earlier. Now the grammar uses values from the bison stack. However, I was making the list rule return the SELECT_LEX :: item_list. This was fixed after the intermediate review. I changed the grammar for INSERT...RETURNING and REPLACE...RETURNING, added test and result files for REPLACE...RETURNING and modified the comments of one test file. I also worked on the intermediate review. Now the parser code doesn't rely on SELECT_LEX :: item_list. In REPLACE...RETURNING a list was declared inside the scope of "if" and using it outside the scope was illegal. So I moved it to the scope outside of if and made the grammar return: &list if select_lex::item_list is not empty else NULL. My github repo is up to date with latest changes. Regards, Rucha Deodhar

1 0

[Maria-developers] INSERT ... RETURNING intermediate review
by Vicențiu Ciorbaru 21 Jul '19

21 Jul '19

Hi Rucha! I've spent quite a bit of time looking at the implementation. It is working and passes almost all test cases, although you forgot to also commit the insert_parser updated result file. Good job so far. I want to give you something to work on and think about for a bit before we do another overall overview of the whole project. Hopefully this will help you across all your coding endeavours, not just now. What I want you to learn is that it is important to think about how your code can and will evolve in the future. Just getting things to work is ok up to a certain point, but not implementing things in a decoupled fashion is what we call "hacks". These hacks are to be avoided as much as possible as it leads to much more difficult work down the line. Let's elaborate on the topic. There are a number of problems with our current code (hacks). I am not referring to yours in particular. All these hacks are a bit difficult to overcome in one go. I do not expect you to fix them yourself, not during this GSoC at least, but it is something to consider and if you are willing to work towards fixing some of them, excellent! Historically our codebase evolved from a simple parser to the "monster" that you see now. Sometimes due to time constraints, things were not implemented in the most extendable way, rather they made use of certain particular constructs, only valid in the contexts they were written in. Keyword here is context, as you'll soon see. Some of this contain things you have interacted with so far in your project. Here is an example that you've had to deal with: A SELECT statement has what we call a SELECT LIST, this list is traditionally stored in SELECT_LEX::item_list. This item_list however is overloaded at times. For example DELETE ... RETURNING uses it to store the expressions part of the RETURNING clause. This overloading generally works ok, unless you run into situations like you did with your INSERT ... RETURNING project. INSERT ... RETURNING clauses already made use of the item_list to store something else. Now you were forced to introduce another list in SELECT_LEX, which we called returning_list. Everything ok so far. But then, you ran into another problem, the bison grammar rules do not put Items in the list you want. They always used item_list, so you could not use your returning_list. So what did we do then? Well, we came up with another hack! We suggested this hack to you because it is something that will work quickly and get you unstuck and that is to use the same grammar rules like before, but swap the item_list with the returning list temporarily, such that the grammar rules will put the right items in the right list. This works, but it masks the underlying problem. The problem is that we have a very rigid implementation for select_item_list grammar rule. What we should do is to make select_item_list grammar return a list using the bison stack. That means no relying on SELECT_LEX::item_list to store our result temporarily. You have done steps towards fixing this, which brings us to where your code's current state. The problem with your implementation is you have not lifted the restriction of the grammar rules making use of SELECT_LIST::item_list. Instead, you have masked it by returning the address of that member on the bison stack. The funny part is that this works, but it still a hack. It is still a hack, because these grammar rules have a side effect of modifying this member behind the scenes. A very, very, very (!) good rule is to consider all side effects as bad, especially now that you are starting out. With experience you might get away with them every now-and-then, but for now avoid them like the plague and try to remove them whenever possible. The current code makes use of a function I really don't like. The inline function add_item_list(). It is one of those hacky functions that strictly relies on context. The context is: take the thd->lex->current_select and put the item passed as a parameter in the item_list. The diff I've attached shows an implementation of how to use the bison stack properly. I took inspiration from the "expr" rule. Unfortunately the hack chain goes deeper, but we need to do baby steps and some things are best left outside your GSoC project, or we risk going down a rabbit whole we might not get out of. One other issue I've observed is with the REPLACE code here: insert_field_spec { if ($6) { List<Item> list; list=*($6); Lex->current_select- >item_list.empty(); $6=&list; } } opt_ select_expressions { if ($8) Lex- >returning_list=*($8); if ($6) Lex->current_select- >item_list=*($6); Lex->pop_select(); //main select if (Lex->check_main_unit_semantics()) MYSQL_YYABORT; } This is really problematic and your probably wrote it because of opt_select_expressions putting stuff into item_list. In the insert_field_spec rule, you receive the address of current_select->item_list as parameter $6. You store the value of this list (basically the pointer to the head element) in a temporary variable that is only in scope during the if statement (!). Attempting to use it after the if statement is over (via the $6 parameter) is illegal! It only works because the compiler did not chose to use that area of memory for something else. Please find a way to store things here properly. After this is fixed, we'll spend the next month cleaning everything up, documenting our code, the added test cases etc. We have an overall working solution, but we need to make it as hack-free as possible, so we can later work on supporting INSERT ... RETURNING in subqueries. Let me know if anything in the email is unclear. It took me a while to write it, trying to explain the reasoning, but I may have missed a few things. :) Vicențiu

2 2

Re: [Maria-developers] MDEV-19429: Wrong query result with EXISTS and LIMIT 0
by Sergey Petrunia 19 Jul '19

19 Jul '19

Hi Sanja, Ok to push after the below input is addressed. > commit ab5fa406b4b314705cb87ffd74111a518b549ff4 > Author: Oleksandr Byelkin <sanja(a)mariadb.com> > Date: Wed Jul 17 12:31:45 2019 +0200 > > MDEV-19429: Wrong query result with EXISTS and LIMIT 0 > > Check EXISTS LIMIT before rewriting. > > diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc > index afc42dc08d5..85d91181337 100644 > --- a/sql/item_subselect.cc > +++ b/sql/item_subselect.cc > @@ -1432,12 +1432,18 @@ void Item_exists_subselect::fix_length_and_dec() > { > DBUG_ENTER("Item_exists_subselect::fix_length_and_dec"); > init_length_and_dec(); > - /* > - We need only 1 row to determine existence (i.e. any EXISTS that is not > - an IN always requires LIMIT 1) > - */ > - thd->change_item_tree(&unit->global_parameters->select_limit, > - new Item_int((int32) 1)); > + // If limit is not set or it is constant more than 1 > + if (!unit->global_parameters->select_limit || > + (unit->global_parameters->select_limit->basic_const_item() && > + unit->global_parameters->select_limit->val_int() > 1)) > + { > + /* > + We need only 1 row to determine existence (i.e. any EXISTS that is not > + an IN always requires LIMIT 1) > + */ > + thd->change_item_tree(&unit->global_parameters->select_limit, > + new Item_int((int32) 1)); Please fix identation ^ > + } > DBUG_PRINT("info", ("Set limit to 1"));a Please move the DBUG_PRINT into the if () {...} . Because right now it will print "set limit to 1" even when it didn't set it. > DBUG_VOID_RETURN; > } I also observe that LIMIT clause is not printed into EXPLAIN EXTENDED output: mysql> explain extended select * from t10 where exists (select * from one_k where a >55 order by a limit 100 offset 50); +------+-------------+-------+------+---------------+------+---------+------+------+----------+-----------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | filtered | Extra | +------+-------------+-------+------+---------------+------+---------+------+------+----------+-----------------------------+ | 1 | PRIMARY | t10 | ALL | NULL | NULL | NULL | NULL | 3 | 100.00 | | | 2 | SUBQUERY | one_k | ALL | NULL | NULL | NULL | NULL | 1342 | 100.00 | Using where; Using filesort | +------+-------------+-------+------+---------------+------+---------+------+------+----------+-----------------------------+ 2 rows in set, 1 warning (6.34 sec) mysql> show warnings\G *************************** 1. row *************************** Level: Note Code: 1003 Message: select `test`.`t10`.`a` AS `a` from `test`.`t10` where exists(select 1 from `test`.`one_k` where (`test`.`one_k`.`a` > 55) order by `test`.`one_k`.`a`) 1 row in set (0.00 sec) ^^^ Note the lack of LIMIT above. It's missing only for EXISTS subqueries, for other kinds of subqueries it is there. I guess this is outside of scope of this MDEV and should be filed separately. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] Interaction between rpl_slave_state and rpl_binlog_state
by Kristian Nielsen 18 Jul '19

18 Jul '19

Andrei Elkin <andrei.elkin(a)mariadb.com> writes: > I would also raise another but relevant topic of maintaining > > gtid_"executed"_pos > > which is an union of all GTID executed regardless of their arrival > method. E.g some of foreign (to the recipient server) domains gtid:s may > The master potentially could be (made) interested in such table > should create a replication mode without necessary binlogging on the > server. Sorry, I don't follow. Do you mean to create a new table of GTID positions, which will be updated by all transactions, whether originating locally or replicated by a slave thread? Or do you mean to have the mysql.gtid_slave_pos table updated also by locally originated transactions? Or do you mean to have a new system variable @@gtid_executed_pos which is constructed from the existing binlog and mysql.gtid_slave_pos_table, similar to @@gtid_current_pos, but maybe including more GTIDs somehow? > This is not really something new as it's exactly how mysql implemented > gtid bookkeeping. I think MySQL originally kept track of GTIDs only in the binlog, right? And binlog was required to be enabled for GTID to work? I do remember seeing something that relaxed this requirement, but I haven't followed the details. - Kristian.

2 2

Re: [Maria-developers] [Commits] fb619954867: MDEV-15010: Wrong Seconds_Behind_Master when only starting the SQL_Thread
by andrei.elkin＠pp.inet.fi 16 Jul '19

16 Jul '19

Sujatha, howdy. Thanks for a good piece of work! The fact that it's actually an upstream contribution only pleases more :-), and to that regard we need a commit id to refer to (and to the people behind that work :-)). > revision-id: fb619954867232b3a733213a2703b2a425dff8e9 (mariadb-5.5.64-19-gfb619954867) > parent(s): 7a7d9904e12335ee8b1eea9671138b3c469a3829 > author: Sujatha > committer: Sujatha > timestamp: 2019-06-19 14:48:18 +0530 > message: > > MDEV-15010: Wrong Seconds_Behind_Master when only starting the SQL_Thread > > Problem: > ======= > When I run replication normally, I get a value in "Seconds_Behind_Master" from > "SHOW SLAVE STATUS". However, when I run only the SQL Thread (on local relay > logs that have been downloaded previously), I have "NULL" in > "Seconds_Behind_Master". I would expect to have a numeric value. > > Fix: > === > Implemented following changes. > case 1: "Seconds_Behind_Master" shows 0, when SQL thread is in sync with IO > thread and IO thread is running. > case 2: "Seconds_Behind_Master" reports NULL, when SQL thread is in sync with > IO thread and IO thread is stopped. > case 3: "Seconds_Behind_Master" reports NULL, when SQL thread is stopped while > IO thread is up and running. > case 4: "Seconds_Behind_Master" reports a valid numerical value when IO thread > is stopped and SQL thread is consuming existing relay log. I have a question to the case 4 in the tests actually. > > --- > .../suite/rpl/r/rpl_seconds_behind_master.result | 40 ++++++++++ > .../suite/rpl/t/rpl_seconds_behind_master.test | 92 ++++++++++++++++++++++ > sql/slave.cc | 82 ++++++++++++------- > 3 files changed, 187 insertions(+), 27 deletions(-) > > diff --git a/mysql-test/suite/rpl/r/rpl_seconds_behind_master.result b/mysql-test/suite/rpl/r/rpl_seconds_behind_master.result > new file mode 100644 > index 00000000000..1c0ce7c1b19 > --- /dev/null > +++ b/mysql-test/suite/rpl/r/rpl_seconds_behind_master.result > @@ -0,0 +1,40 @@ > +include/master-slave.inc > +[connection master] > +CREATE TABLE t1 (a INT PRIMARY KEY, b INT); > +****************************************************************** > +* Case1: SQL thread is in sync with IO thread and IO thread is UP > +* Seconds_Behind_Master should be '0'. > +****************************************************************** > +include/assert.inc [Seconds_Behind_Master should be 0] > +****************************************************************** > +* Case2: SQL thread is in sync with IO thread and IO thread is > +* stopped. > +* Seconds_Behind_Master should be 'NULL'. > +****************************************************************** > +include/stop_slave_io.inc > +include/assert.inc [Seconds_Behind_Master should be NULL] > +****************************************************************** > +* Case3: SQL thread is stopped while IO thread is up and running. > +* Seconds_Behind_Master should be 'NULL'. > +****************************************************************** > +START SLAVE IO_THREAD; > +include/wait_for_slave_io_to_start.inc > +include/stop_slave_sql.inc > +include/assert.inc [Seconds_Behind_Master should be NULL] > +****************************************************************** > +* Case4: IO thread is stopped. SQL thread is consuming existing relay > +* log. Hence the Seconds_Behind_Master should have a numeric > +* value > 0 > +****************************************************************** > +INSERT INTO t1 VALUES (1,sleep(2)); > +INSERT INTO t1 VALUES (2,sleep(2)); > +include/sync_slave_io_with_master.inc > +include/stop_slave_io.inc > +START SLAVE SQL_THREAD; > +include/wait_for_slave_sql_to_start.inc > +include/assert.inc [Seconds_Behind_Master should be > 0] > +START SLAVE IO_THREAD; > +include/wait_for_slave_io_to_start.inc > +******* CLEANUP ********* > +DROP TABLE t1; > +include/rpl_end.inc > diff --git a/mysql-test/suite/rpl/t/rpl_seconds_behind_master.test b/mysql-test/suite/rpl/t/rpl_seconds_behind_master.test > new file mode 100644 > index 00000000000..6f9ed2939e1 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_seconds_behind_master.test > @@ -0,0 +1,92 @@ > +# ==== Purpose ==== > +# > +# Test verifies that Seconds_Behind_Master(SBM) values are as expected in various > +# scenario listed below. > +# > +# case 1: SBM=0 if SQL thread is in sync with IO thread and IO thread is UP > +# case 2: SBM=NULL if SQL thread is in sync with IO thread and IO thread is > +# stopped. > +# case 3: SBM=NULL if SQL thread is stopped while IO thread is up and running. > +# case 4: SBM > 0 and SBM != NULL if IO thread is stopped. SQL thread is > +# consuming existing relay log. > +# > +# ==== Implementation ==== > +# > +# Steps: > +# 1 - Create a table and sync it with slave and verify SBM value. > +# 2 - Now stop IO thread and verify SBM > +# 3 - Start IO thread, Stop SQL thread and verify SBM > +# 4 - Execute slow INSERT statements with SLEEP(2) on master. Ensure that > +# the IO thread has read these INSERT statements and then bring it down. > +# 5 - Start the SQL thread and verify that while it is trying to consume > +# these slow INSERT statements in relay log the SBM value is > 0. > +# > +# ==== References ==== > +# > +# MDEV-15010: Wrong Seconds_Behind_Master when only starting the SQL_Thread. > + > +--source include/have_binlog_format_statement.inc > +--source include/master-slave.inc > + > +--disable_query_log > +CALL mtr.add_suppression("Unsafe statement written to the binary log using statement format since BINLOG_FORMAT = STATEMENT"); > +--enable_query_log > + > +CREATE TABLE t1 (a INT PRIMARY KEY, b INT); > +--sync_slave_with_master > + > +-- echo ****************************************************************** > +-- echo * Case1: SQL thread is in sync with IO thread and IO thread is UP > +-- echo * Seconds_Behind_Master should be '0'. > +-- echo ****************************************************************** > +--let $assert_cond= [SHOW SLAVE STATUS, Seconds_Behind_Master, 1] = 0 > +--let $assert_text= Seconds_Behind_Master should be 0 > +--source include/assert.inc > + > +-- echo ****************************************************************** > +-- echo * Case2: SQL thread is in sync with IO thread and IO thread is > +-- echo * stopped. > +-- echo * Seconds_Behind_Master should be 'NULL'. > +-- echo ****************************************************************** > +--source include/stop_slave_io.inc > +--let $assert_cond= "[SHOW SLAVE STATUS, Seconds_Behind_Master, 1]" = "NULL" > +--let $assert_text= Seconds_Behind_Master should be NULL > +--source include/assert.inc > + > +-- echo ****************************************************************** > +-- echo * Case3: SQL thread is stopped while IO thread is up and running. > +-- echo * Seconds_Behind_Master should be 'NULL'. > +-- echo ****************************************************************** > +START SLAVE IO_THREAD; > +--source include/wait_for_slave_io_to_start.inc > +--source include/stop_slave_sql.inc > +--let $assert_cond= "[SHOW SLAVE STATUS, Seconds_Behind_Master, 1]" = "NULL" > +--let $assert_text= Seconds_Behind_Master should be NULL > +--source include/assert.inc > + > +-- echo ****************************************************************** > +-- echo * Case4: IO thread is stopped. SQL thread is consuming existing relay > +-- echo * log. Hence the Seconds_Behind_Master should have a numeric > +-- echo * value > 0 > +-- echo ****************************************************************** > +--connection master > +--disable_warnings > +INSERT INTO t1 VALUES (1,sleep(2)); > +INSERT INTO t1 VALUES (2,sleep(2)); > +--enable_warnings > +--source include/sync_slave_io_with_master.inc > +--source include/stop_slave_io.inc > + > +START SLAVE SQL_THREAD; > +--source include/wait_for_slave_sql_to_start.inc There's a theoretical (and even though I think it would hardly ever be practical) chance for the SQL thread to catch up while for some concurrency reason the waiting would take longer that it's naturally expected. How about to + LOCK TABLES t1 WRITE before START SLAVE SQL_THREAD and unlock > +--let $assert_cond= [SHOW SLAVE STATUS, Seconds_Behind_Master, 1] > 0 > +--let $assert_text= Seconds_Behind_Master should be > 0 > +--source include/assert.inc after the assert check? This measure would speak rather explicitly that the check is done just before any first event has been processed by the SQL thread, and as you prepare 2, then no synchronization is guaranteed. Cheers, Andrei > +START SLAVE IO_THREAD; > +--source include/wait_for_slave_io_to_start.inc > + > +--echo ******* CLEANUP ********* > +--connection master > +DROP TABLE t1; > + > +--source include/rpl_end.inc > diff --git a/sql/slave.cc b/sql/slave.cc > index a8946c69d18..7b4b7bd857e 100644 > --- a/sql/slave.cc > +++ b/sql/slave.cc > @@ -2192,38 +2192,66 @@ bool show_master_info(THD* thd, Master_info* mi) > protocol->store(mi->ssl_cert, &my_charset_bin); > protocol->store(mi->ssl_cipher, &my_charset_bin); > protocol->store(mi->ssl_key, &my_charset_bin); > - > /* > - Seconds_Behind_Master: if SQL thread is running and I/O thread is > - connected, we can compute it otherwise show NULL (i.e. unknown). > + The pseudo code to compute Seconds_Behind_Master: > + if (SQL thread is running) > + { > + if (SQL thread processed all the available relay log) > + { > + if (IO thread is running) > + print 0; > + else > + print NULL; > + } > + else > + compute Seconds_Behind_Master; > + } > + else > + print NULL; > */ > - if ((mi->slave_running == MYSQL_SLAVE_RUN_CONNECT) && > - mi->rli.slave_running) > + if (mi->rli.slave_running) > { > - long time_diff= ((long)(time(0) - mi->rli.last_master_timestamp) > - - mi->clock_diff_with_master); > /* > - Apparently on some systems time_diff can be <0. Here are possible > - reasons related to MySQL: > - - the master is itself a slave of another master whose time is ahead. > - - somebody used an explicit SET TIMESTAMP on the master. > - Possible reason related to granularity-to-second of time functions > - (nothing to do with MySQL), which can explain a value of -1: > - assume the master's and slave's time are perfectly synchronized, and > - that at slave's connection time, when the master's timestamp is read, > - it is at the very end of second 1, and (a very short time later) when > - the slave's timestamp is read it is at the very beginning of second > - 2. Then the recorded value for master is 1 and the recorded value for > - slave is 2. At SHOW SLAVE STATUS time, assume that the difference > - between timestamp of slave and rli->last_master_timestamp is 0 > - (i.e. they are in the same second), then we get 0-(2-1)=-1 as a result. > - This confuses users, so we don't go below 0: hence the max(). > - > - last_master_timestamp == 0 (an "impossible" timestamp 1970) is a > - special marker to say "consider we have caught up". > + Check if SQL thread is at the end of relay log > + Checking should be done using two conditions > + condition1: compare the log positions and > + condition2: compare the file names (to handle rotation case) > */ > - protocol->store((longlong)(mi->rli.last_master_timestamp ? > - max(0, time_diff) : 0)); > + if ((mi->master_log_pos == mi->rli.group_master_log_pos) && > + (!strcmp(mi->master_log_name, mi->rli.group_master_log_name))) > + { > + if (mi->slave_running == MYSQL_SLAVE_RUN_CONNECT) > + protocol->store(0LL); > + else > + protocol->store_null(); > + } > + else > + { > + long time_diff = ((long)(time(0) - mi->rli.last_master_timestamp) > + - mi->clock_diff_with_master); > + /* > + Apparently on some systems time_diff can be <0. Here are possible > + reasons related to MySQL: > + - the master is itself a slave of another master whose time is ahead. > + - somebody used an explicit SET TIMESTAMP on the master. > + Possible reason related to granularity-to-second of time functions > + (nothing to do with MySQL), which can explain a value of -1: > + assume the master's and slave's time are perfectly synchronized, and > + that at slave's connection time, when the master's timestamp is read, > + it is at the very end of second 1, and (a very short time later) when > + the slave's timestamp is read it is at the very beginning of second > + 2. Then the recorded value for master is 1 and the recorded value for > + slave is 2. At SHOW SLAVE STATUS time, assume that the difference > + between timestamp of slave and rli->last_master_timestamp is 0 > + (i.e. they are in the same second), then we get 0-(2-1)=-1 as a result. > + This confuses users, so we don't go below 0: hence the max(). > + > + last_master_timestamp == 0 (an "impossible" timestamp 1970) is a > + special marker to say "consider we have caught up". > + */ > + protocol->store((longlong)(mi->rli.last_master_timestamp ? > + max(0, time_diff) : 0)); > + } > } > else > { > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

[Maria-developers] MDEV-8306: more input
by Sergey Petrunia 16 Jul '19

16 Jul '19

Hi Varun, Please find more input, I'm commenting on the second patch (the review is probably incomplete, I've made just one pass so might have missed something) > diff -urpN '--exclude=.*' 10.5-mdev8306-orig/sql/opt_subselect.cc 10.5-mdev8306/sql/opt_subselect.cc > --- 10.5-mdev8306-orig/sql/opt_subselect.cc 2019-07-10 11:30:41.151696943 +0300 > +++ 10.5-mdev8306/sql/opt_subselect.cc 2019-07-16 13:07:13.948467315 +0300 > @@ -5517,6 +5517,31 @@ enum_nested_loop_state join_tab_executio > sjm->materialized= TRUE; > } > } > + /* > + This should be the place to add the sub_select call for the > + prefix of the join order > + */ > + > + else if (tab->is_order_nest) > + { > + enum_nested_loop_state rc; > + JOIN *join= tab->join; > + NEST_INFO *nest_info= join->order_nest_info; > + > + if (!nest_info->materialized) > + { > + JOIN_TAB *join_tab= join->join_tab + join->const_tables; > + JOIN_TAB *save_return_tab= join->return_tab; > + if ((rc= sub_select(join, join_tab, FALSE)) < 0 || > + (rc= sub_select(join, join_tab, TRUE)) < 0) > + { > + join->return_tab= save_return_tab; > + DBUG_RETURN(rc); > + } > + join->return_tab= save_return_tab; > + nest_info->materialized= TRUE; > + } Looks a bit repetitive with the SJM nest code above, but this is not a big deal. A question: if the JOIN_TABs for the tables inside the sort nest are still in the join->join_tab array, where is the code to "jump over" them in regular join execution? > + } > > DBUG_RETURN(NESTED_LOOP_OK); > } ... > diff -urpN '--exclude=.*' 10.5-mdev8306-orig/sql/sql_class.h 10.5-mdev8306/sql/sql_class.h > --- 10.5-mdev8306-orig/sql/sql_class.h 2019-07-16 13:10:05.448491384 +0300 > +++ 10.5-mdev8306/sql/sql_class.h 2019-07-16 13:07:13.948467315 +0300 > @@ -6037,6 +6037,24 @@ public: > Copy_field *copy_field; /* Needed for SJ_Materialization scan */ > }; > > +class NEST_INFO : public Sql_alloc > +{ > +public: > + ~NEST_INFO() > + { > + nest_tab= NULL; > + table= NULL; > + } What is this for? Object members should not be used after destructor is called. If you're concerned about accidental access, please use TRASH_XXX macros to fill the variables with "already-freed" marker. > + TMP_TABLE_PARAM tmp_table_param; > + List<Item> nest_base_table_cols; > + List<Item> nest_temp_table_cols; > + TABLE *table; > + st_join_table *nest_tab; > + uint n_tables; > + bool materialized; /* TRUE <=> materialization already performed */ > + table_map nest_tables_map; > +}; > + > > /* Structs used when sorting */ > struct SORT_FIELD_ATTR ... > diff -urpN '--exclude=.*' 10.5-mdev8306-orig/sql/sql_select.cc 10.5-mdev8306/sql/sql_select.cc > --- 10.5-mdev8306-orig/sql/sql_select.cc 2019-07-16 13:10:05.452491478 +0300 > +++ 10.5-mdev8306/sql/sql_select.cc 2019-07-16 13:07:13.952467409 +0300 > return retval; > } > > +void substitute_base_to_nest_items(JOIN *join) > +{ > + NEST_INFO *order_nest_info= join->order_nest_info; > + if (!order_nest_info) > + return; > + REPLACE_NEST_FIELD_ARG arg= {join}; > + join->conds= join->conds->transform(join->thd, &Item::replace_with_nest_items, > + (uchar *) &arg); > + > + List_iterator<Item> it(join->fields_list); > + Item *item, *new_item; > + uint i=0; > + while ((item= it++)) > + { > + if ((new_item= item->transform(join->thd, > + &Item::replace_with_nest_items, > + (uchar *) &arg)) != item) > + it.replace(new_item); > + } Ok, so you are processing the select list, and the WHERE condition. What about the ON expressions, and the right sides of ref access? Also, how do we solve the issue of the same Item_field object being used in two conditions at once: - one inside the sort nest - one outside the sort nest (remember the example with " (inner_tbl_cond1 AND outer_tbl_con1) OR (inner_tbl_cond2 AND outer_tbl_cond2) " > +} > + > > /** ... > @@ -12136,6 +12314,35 @@ end_sj_materialize(JOIN *join, JOIN_TAB > } > > > +enum_nested_loop_state > +end_nest_materialization(JOIN *join, JOIN_TAB *join_tab, bool end_of_records) > +{ > + int error; > + THD *thd= join->thd; > + NEST_INFO *nest_info= join->order_nest_info; > + DBUG_ENTER("end_sj_materialize"); Please change to match the function name. > + if (!end_of_records) > + { > + TABLE *table= nest_info->table; > + fill_record(thd, table, table->field, > + nest_info->nest_base_table_cols, TRUE, FALSE); > + > + if (unlikely(thd->is_error())) > + DBUG_RETURN(NESTED_LOOP_ERROR); /* purecov: inspected */ > + if (unlikely((error= table->file->ha_write_tmp_row(table->record[0])))) > + { > + /* create_myisam_from_heap will generate error if needed */ > + if (table->file->is_fatal_error(error, HA_CHECK_DUP) && Is this check necessary? The table doesn't have any unique keys, why do we ignore duplicates? > + create_internal_tmp_table_from_heap(thd, table, > + nest_info->tmp_table_param.start_recinfo, > + &nest_info->tmp_table_param.recinfo, > + error, 1, NULL)) > + DBUG_RETURN(NESTED_LOOP_ERROR); /* purecov: inspected */ > + } > + } > + DBUG_RETURN(NESTED_LOOP_OK); > +} > + > /* > Check whether a join buffer can be used to join the specified table > BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0