March 2019 - developers - lists.mariadb.org

[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] 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

Re: [Maria-developers] 7198e7c71fc: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
by Sergei Golubchik 13 May '19

13 May '19

Hi, Aleksey! On Dec 23, Aleksey Midenkov wrote: > revision-id: 7198e7c71fc (versioning-1.0.6-82-g7198e7c71fc) > parent(s): 6be155757b7 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-12-20 13:25:00 +0300 > message: > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > * Versioning tests support As far as I can see, there are two approaches to fixing this bug. And if we go the way of HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS then your fix is good. But I'd still like to understand whether write_row is supposed to disturb the scan. Why should heap_write() overwrite info->current_ptr=pos and info->current_hash_ptr? Inserting a row doesn't happen at the "current position", so I don't quite understand why it modifies current_ptr at all. There is no functionality like "continue the scan from the last inserted row", right? It just doesn't make sense to me. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

[Maria-developers] Buildbot image update request
by Otto Kekäläinen 01 Apr '19

01 Apr '19

Hello! Can you please update the image used by test kvm-deb-jessie-ppc64le so that the 'apt-get update' is fresh and package lists are recent? It would fix the error in http://buildbot.askmonty.org/buildbot/builders/kvm-deb-jessie-ppc64le/build… E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/m/mpfr4/libmpfr4_3.1.2-2_ppc64el.… 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/g/gcc-4.9/libatomic1_4.9.2-10_ppc… 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/r/rsync/rsync_3.1.1-3_ppc64el.deb 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/s/socat/socat_1.7.2.4-2_ppc64el.d… 404 Not Found [IP: 64.50.233.100 80] E: Failed to fetch http://ftp.us.debian.org/debian/pool/main/libd/libdbd-mysql-perl/libdbd-mys… 404 Not Found [IP: 64.50.233.100 80] Also, can you please run 'apt-get install libzstd-dev' in the image used in arm64-xenial-deb? It would fix http://buildbot.askmonty.org/buildbot/builders/arm64-xenial-deb/builds/2973… dpkg-checkbuilddeps: error: Unmet build dependencies: libzstd-dev dpkg-buildpackage: warning: build dependencies/conflicts unsatisfied; aborting Thanks!

2 1

Re: [Maria-developers] 9976699033c: Test fixes (versioning.replace, versioning.foreign)
by Sergei Golubchik 31 Mar '19

31 Mar '19

Hi, Aleksey! On Mar 27, Aleksey Midenkov wrote: > revision-id: 9976699033c (mariadb-10.3.7-30-g9976699033c) > parent(s): c0f97710582 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-05-30 13:12:47 +0300 > message: > > Test fixes (versioning.replace, versioning.foreign) > > * replace.test: unwrapped create_table procedure; > * foreign.test: use key_type combinations. > > diff --git a/mysql-test/suite/versioning/key_type.combinations b/mysql-test/suite/versioning/key_type.combinations > index 1929aee9a84..fb3888d3d0f 100644 > --- a/mysql-test/suite/versioning/key_type.combinations > +++ b/mysql-test/suite/versioning/key_type.combinations > @@ -1,2 +1,3 @@ > [unique] > -[pk] > +[primary] > +[secondary] generally prefer shorter combination names, otherwise mtr output becomes garbled and difficult to read. > diff --git a/mysql-test/suite/versioning/primary_or_unique.inc b/mysql-test/suite/versioning/primary_or_unique.inc > new file mode 100644 > index 00000000000..199d8bab942 > --- /dev/null > +++ b/mysql-test/suite/versioning/primary_or_unique.inc > @@ -0,0 +1,5 @@ > +--source suite/versioning/key_type.inc > +--require suite/versioning/primary_or_unique.require don't do .require, please, it's a technique from 1998, before mysqltest had the "skip" command. Use if (...) { skip; } > +disable_query_log; > +eval select "$KEY_TYPE" != "key" as pk_or_unique; > +enable_query_log; > diff --git a/mysql-test/suite/versioning/t/engines.combinations b/mysql-test/suite/versioning/t/engines.combinations > deleted file mode 100644 > index 561c5656929..00000000000 > --- a/mysql-test/suite/versioning/t/engines.combinations > +++ /dev/null > @@ -1,8 +0,0 @@ > -[timestamp] > -default-storage-engine=innodb > - > -[trx_id] > -default-storage-engine=innodb > - > -[myisam] > -default-storage-engine=myisam Is that intentional? I thought many tests use it to run in different versioning modes. > diff --git a/mysql-test/suite/versioning/t/foreign.test b/mysql-test/suite/versioning/t/foreign.test > index 566d481c2a8..72e5b047cbb 100644 > --- a/mysql-test/suite/versioning/t/foreign.test > +++ b/mysql-test/suite/versioning/t/foreign.test > @@ -1,11 +1,14 @@ > +--source suite/versioning/primary_only.inc > --source suite/versioning/common.inc what's the point of this? if you're only allowing primary key here, you can just as well write manually "primary key" in all tests, without $KEY_TYPE and replace_result. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

[Maria-developers] EITS member variables in both TABLE and TABLE_SHARE?
by Sergey Petrunia 29 Mar '19

29 Mar '19

Hello Igor, TABLE_SHARE has TABLE_STATISTICS_CB, which has these members: struct TABLE_STATISTICS_CB { ... bool stats_is_read; /* Statistical data for table has been read from statistical tables */ bool histograms_are_read; } At the same time, TABLE has: struct TABLE { ... bool stats_is_read; /* Persistent statistics is read for the table */ bool histograms_are_read; I would like to understand why both structures need these variables? Is there any difference in meaning between them? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 74b2eba1ca6: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
by Sergei Golubchik 29 Mar '19

29 Mar '19

Hi, Aleksey! On Mar 26, Aleksey Midenkov wrote: > revision-id: 74b2eba1ca6 (mariadb-10.3.12-115-g74b2eba1ca6) > parent(s): 2c0901d808b > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Sergei Golubchik <serg(a)mariadb.com> > timestamp: 2019-03-26 17:43:38 +0100 > message: > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > * Versioning tests support > > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index abe4254db3e..1d51947cc86 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -1643,7 +1643,12 @@ int vers_insert_history_row(TABLE *table) > if (row_start->cmp(row_start->ptr, row_end->ptr) >= 0) > return 0; > > - return table->file->ha_write_row(table->record[0]); > + int res; > + if ((res= table->file->extra(HA_EXTRA_REMEMBER_POS))) > + return res; > + if ((res= table->file->ha_write_row(table->record[0]))) > + return res; > + return table->file->extra(HA_EXTRA_RESTORE_POS); > } Frankly, I don't like it. I mean, this particular fix is fine, but here's the problem: We now have (at least) three features where the server might need to do something (insert/update/search) during the index/rnd scan. And three different solutions: you use HA_EXTRA_REMEMBER_POS in system versioning, Nikita simply sets delete_while_scanning=false in application time period tables, and Sachin uses handler::clone() to create a second handler that can be used to check for hash collisions without disrupting the scan on the primary handler. I think it's getting somewhat out of hands. Can we please reduce the number of approaches to fix the same issue? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

4 4

Re: [Maria-developers] ab6ef429a84: MDEV-16370 row-based binlog events (updates by primary key) can not be applied multiple times to system versioned tables
by Sergei Golubchik 29 Mar '19

29 Mar '19

Hi, Aleksey! On Jan 31, Aleksey Midenkov wrote: > revision-id: ab6ef429a84 (versioning-1.0.7-2-gab6ef429a84) > parent(s): 14c2f90ad08 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-11-26 12:46:37 +0300 > message: > > MDEV-16370 row-based binlog events (updates by primary key) can not be applied multiple times to system versioned tables > > Case 2: > > * Idempotent BINLOG updates: duplicate key on history row is skipped; > * Fix row start and warn when applying BINLOG from the past. As far as I can see now you cannot just arbitrarily adjust row_start. Think of it in term or application time period tables, a primary key is (kind of) defined WITHOUT OVERLAPS. There is no explicit constraint, it happens automatically when the history is valid. Now, here's your test case: timestamp=100; insert (1,1) -> the table has (1,1,100,MAX); timestamp=300; update to (1,3) -> (1,1,100,300),(1,3,300,MAX); timestamp=200; update to (1,2) -> here the hisory row should've been (1,3,300,200) but you adjust it to get 1,1,100,300 1,3,199,200 1,2,200,MAX and if you select AS OF 199, you'll get two rows for the pk=1 So, let's * ignore HA_ERR_FOUND_DUPP_KEY (please check if it's needed for insert or delete too) * add TIMESTAMP=xxx to mysqldumps. can vers_insert_history_row see row_start>row_end in a normal use case? no replication or any other way of adjusting the timestamp. equality can surely happen, but greater than ? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

[Maria-developers] THD::set_time()
by Aleksey Midenkov 28 Mar '19

28 Mar '19

What if system_time.sec_part == TIME_MAX_SECOND_PART here: else { start_time= t; start_time_sec_part= ++system_time.sec_part; } ? -- All the best, Aleksey Midenkov @midenok

1 0

[Maria-developers] (no subject)
by Samyak Jain 28 Mar '19

28 Mar '19

I want to contribute for full outer join gsoc project of mariaDB. Please provide the technical requirements for this and how I start contributing on this

1 0

Re: [Maria-developers] c1bf5766aed: MDEV-16490: It's possible to make a system versioned table without any versioning field
by Sergei Golubchik 27 Mar '19

27 Mar '19

Hi, Nikita! On Mar 27, Nikita Malyavin wrote: > revision-id: c1bf5766aed (versioning-1.0.5-41-gc1bf5766aed) > parent(s): 141a5b24843 > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Nikita Malyavin <nikitamalyavin(a)gmail.com> > timestamp: 2018-07-20 23:37:26 +1000 > message: > > MDEV-16490: It's possible to make a system versioned table without any versioning field > > * do not allow versioned table to be without versioned (non-system) fields > * prohibit changing field versioning, when removing table versioning > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index dbf400b53d7..ee4cf457f45 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -8473,11 +8473,21 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, > } > } > > - if (table->versioned() && !(alter_info->flags & ALTER_DROP_SYSTEM_VERSIONING) && > - new_create_list.elements == VERSIONING_FIELDS) > + if (table->versioned()) > { > - my_error(ER_VERS_TABLE_MUST_HAVE_COLUMNS, MYF(0), table->s->table_name.str); > - goto err; > + uint versioned_fields= 0; > + field_it.rewind(); > + while (Create_field *f= field_it++) > + { > + if (!(f->flags & VERS_UPDATE_UNVERSIONED_FLAG)) > + versioned_fields++; > + } > + if (!(alter_info->flags & ALTER_DROP_SYSTEM_VERSIONING) && > + versioned_fields == VERSIONING_FIELDS) > + { > + my_error(ER_VERS_TABLE_MUST_HAVE_COLUMNS, MYF(0), table->s->table_name.str); > + goto err; > + } wouldn't that fit naturally into Vers_parse_info::fix_alter_info() ? Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] e0fa1993692: MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value
by Sergey Petrunia 27 Mar '19

27 Mar '19

Hi Varun, First, Some input on the test: On Wed, Mar 27, 2019 at 01:55:16PM +0530, Varun wrote: > revision-id: e0fa1993692a3552feddae56cbee3078d4da2121 (mariadb-10.2.22-91-ge0fa1993692) > parent(s): 50a8fc52988d13a5164a1a542b9d7a85e3ecc1c1 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2019-03-27 13:48:44 +0530 > message: > > MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value > > To fix the crash there we need to make sure that the > server while storing the statistical values in statistical tables should do it > in a multi-byte safe way. > Also there is no need to throw warnings if there is truncation while storing > values from statistical fields. > ... > --- a/mysql-test/r/stat_tables_innodb.result > +++ b/mysql-test/r/stat_tables_innodb.result > @@ -651,6 +651,63 @@ SELECT MAX(pk) FROM t1; > MAX(pk) > NULL > DROP TABLE t1; > +# > +# MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value > +# > +set names utf8; > +set @save_optimizer_use_condition_selectivity=@@optimizer_use_condition_selectivity; > +set optimizer_use_condition_selectivity=4; > +set use_stat_tables=preferably; > +set @save_histogram_size= @@histogram_size; > +set histogram_size=255; > +create table t1 ( a varchar(255) character set utf8); > +insert into t1 values (REPEAT('ӥ',255)), (REPEAT('ç',255)); > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status OK > +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats; > +HEX(RIGHT(min_value, 1)) length(min_value) > +A7 254 > +select HEX(RIGHT(max_value, 1)), length(max_value) from mysql.column_stats; > +HEX(RIGHT(max_value, 1)) length(max_value) > +A5 254 > +analyze select * from t1 where a >= 'ӥ'; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 2 2.00 50.00 50.00 Using where > +set @save_sql_mode= @@sql_mode; > +set sql_mode='ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION'; > +update mysql.column_stats set min_value= REPEAT('ӥ',255); This looks scary, what if there were some other data in these tables? Can you change add a WHERE db_name=database() and table_name='t1' ? > +Warnings: > +Warning 1265 Data truncated for column 'min_value' at row 1 > +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats; > +HEX(RIGHT(min_value, 1)) length(min_value) > +D3 255 > +analyze select * from t1 where a >= 'ӥ'; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 2 2.00 50.00 50.00 Using where > +set names latin1; > +drop table t1; > +CREATE TABLE t1 (col1 date); > +INSERT INTO t1 VALUES('2004-01-01'),('2004-02-29'); > +INSERT INTO t1 VALUES('0000-10-31'); > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status OK > +update mysql.column_stats set min_value='2004-0-31123'; The same as above. > +select min_value from mysql.column_stats; > +min_value > +2004-0-31123 This case also makes one wonder whether not producing a warning was a good idea after all. A varchar string that is 255-byte long, and after 255 bytes it gets truncated in the middle of a character is a rare case. An incorrect datetime value... not as rare? > +select * from t1; Is the above supposed to read the EITS data? It doesn't as the query doesn't use the WHERE clause. > +col1 > +2004-01-01 > +2004-02-29 > +0000-10-31 > +drop table t1; > +set @@sql_mode= @save_sql_mode; > set use_stat_tables=@save_use_stat_tables; > +set @@histogram_size= @save_histogram_size; > +set @@optimizer_use_condition_selectivity=@save_optimizer_use_condition_selectivity; > set optimizer_switch=@save_optimizer_switch_for_stat_tables_test; > SET SESSION STORAGE_ENGINE=DEFAULT; > diff --git a/mysql-test/t/stat_tables.test b/mysql-test/t/stat_tables.test > index b89ab2bbd2d..114c5c97a7e 100644 > --- a/mysql-test/t/stat_tables.test > +++ b/mysql-test/t/stat_tables.test > @@ -401,4 +401,44 @@ SELECT MAX(pk) FROM t1; > > DROP TABLE t1; > > +--echo # > +--echo # MDEV-18899: Server crashes in Field::set_warning_truncated_wrong_value > +--echo # > + > +set names utf8; > +set @save_optimizer_use_condition_selectivity=@@optimizer_use_condition_selectivity; > +set optimizer_use_condition_selectivity=4; > +set use_stat_tables=preferably; > +set @save_histogram_size= @@histogram_size; > +set histogram_size=255; > + > +create table t1 ( a varchar(255) character set utf8); > +insert into t1 values (REPEAT('ӥ',255)), (REPEAT('ç',255)); > + > +analyze table t1; > +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats; > +select HEX(RIGHT(max_value, 1)), length(max_value) from mysql.column_stats; > +analyze select * from t1 where a >= 'ӥ'; > + > +set @save_sql_mode= @@sql_mode; > +set sql_mode='ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION'; > +update mysql.column_stats set min_value= REPEAT('ӥ',255); > +select HEX(RIGHT(min_value, 1)), length(min_value) from mysql.column_stats; > +analyze select * from t1 where a >= 'ӥ'; > + > +set names latin1; > +drop table t1; > + > +CREATE TABLE t1 (col1 date); > +INSERT INTO t1 VALUES('2004-01-01'),('2004-02-29'); > +INSERT INTO t1 VALUES('0000-10-31'); > +analyze table t1; > +update mysql.column_stats set min_value='2004-0-31123'; > +select min_value from mysql.column_stats; > +select * from t1; > +drop table t1; > + > +set @@sql_mode= @save_sql_mode; > set use_stat_tables=@save_use_stat_tables; > +set @@histogram_size= @save_histogram_size; > +set @@optimizer_use_condition_selectivity=@save_optimizer_use_condition_selectivity; > diff --git a/sql/field.cc b/sql/field.cc > index 080cf34c76d..c26195a3650 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -2219,7 +2219,6 @@ bool Field_str::can_be_substituted_to_equal_item(const Context &ctx, > return false; > } > > - > void Field_num::make_field(Send_field *field) > { > Field::make_field(field); > @@ -7023,12 +7022,16 @@ Field_longstr::check_string_copy_error(const String_copier *copier, > { > const char *pos; > char tmp[32]; > + THD *thd= get_thd(); > > if (!(pos= copier->most_important_error_pos())) > return FALSE; > > - convert_to_printable(tmp, sizeof(tmp), pos, (end - pos), cs, 6); > - set_warning_truncated_wrong_value("string", tmp); > + if (thd->count_cuted_fields) > + { > + convert_to_printable(tmp, sizeof(tmp), pos, (end - pos), cs, 6); > + set_warning_truncated_wrong_value("string", tmp); > + } > return TRUE; > } > > diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc > index b5811c683e8..0960a9a1ec5 100644 > --- a/sql/sql_statistics.cc > +++ b/sql/sql_statistics.cc > @@ -1044,6 +1044,7 @@ class Column_stat: public Stat_table > { > char buff[MAX_FIELD_WIDTH]; > String val(buff, sizeof(buff), &my_charset_bin); > + uint32 length= 0; > > for (uint i= COLUMN_STAT_MIN_VALUE; i <= COLUMN_STAT_HISTOGRAM; i++) > { > @@ -1060,7 +1061,9 @@ class Column_stat: public Stat_table > else > { > table_field->collected_stats->min_value->val_str(&val); > - stat_field->store(val.ptr(), val.length(), &my_charset_bin); > + length= Well_formed_prefix(val.charset(), val.ptr(), > + MY_MIN(val.length(), stat_field->field_length)).length(); > + stat_field->store(val.ptr(), length, &my_charset_bin); > } > break; > case COLUMN_STAT_MAX_VALUE: > @@ -1069,7 +1072,9 @@ class Column_stat: public Stat_table > else > { > table_field->collected_stats->max_value->val_str(&val); > - stat_field->store(val.ptr(), val.length(), &my_charset_bin); > + length= Well_formed_prefix(val.charset(), val.ptr(), > + MY_MIN(val.length(), stat_field->field_length)).length(); > + stat_field->store(val.ptr(), length, &my_charset_bin); > } > break; > case COLUMN_STAT_NULLS_RATIO: > @@ -2934,7 +2939,6 @@ int update_statistics_for_table(THD *thd, TABLE *table) > DBUG_RETURN(rc); > } > > - > /** > @brief > Read statistics for a table from the persistent statistical tables > @@ -2980,6 +2984,7 @@ int read_statistics_for_table(THD *thd, TABLE *table, TABLE_LIST *stat_tables) > Table_statistics *read_stats= table_share->stats_cb.table_stats; > > DBUG_ENTER("read_statistics_for_table"); > + DBUG_ASSERT(thd->count_cuted_fields == CHECK_FIELD_IGNORE); > > /* Read statistics from the statistical table table_stats */ > stat_table= stat_tables[TABLE_STAT].table; > @@ -3059,7 +3064,7 @@ int read_statistics_for_table(THD *thd, TABLE *table, TABLE_LIST *stat_tables) > } > } > } > - > + > table->stats_is_read= TRUE; > > DBUG_RETURN(0); BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 494c0448c87: MDEV-15951 system versioning by trx id doesn't work with partitioning
by Sergei Golubchik 26 Mar '19

26 Mar '19

Hi, Nikita! That's good, thanks! It's always nice to see a fix that removes more code than it adds :) But I'll reformulate the error message a bit, "Transactional system versioned tables" also means InnoDB system versioned by timestamp. I'll clarify (somehow) that it means versioned by trx_id, and push. On Mar 26, Nikita Malyavin wrote: > revision-id: 494c0448c87 (mariadb-10.3.12-65-g494c0448c87) > parent(s): 4946eb7b543 > author: Nikita Malyavin <nikitamalyavin(a)gmail.com> > committer: Nikita Malyavin <nikitamalyavin(a)gmail.com> > timestamp: 2019-02-28 19:29:12 +1000 > message: > > MDEV-15951 system versioning by trx id doesn't work with partitioning > > Fix partitioning for trx_id-versioned tables. > `partition by hash`, `range` and others now work. > `partition by system_time` is forbidden. > Currently we cannot use row_start and row_end in `partition by`, because > insertion of versioned field is done by engine's handler, as well as > row_start/row_end's value set up, which is a transaction id -- so it's > also forbidden. > > The drawback is that it's now impossible to use `partition by key()` > without parameters for such tables, because it references row_start and > row_end implicitly. > > * add handler::vers_can_native() > * drop Table_scope_and_contents_source_st::vers_native() > * drop partition_element::find_engine_flag as unused > * forbid versioning partitioning for trx_id as not supported > * adopt vers tests for trx_id partitioning > * forbid any row_end referencing in `partition by` clauses, > including implicit `by key()` > > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index 9112795b754..dff12ef0a97 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7907,8 +7907,8 @@ ER_UNUSED_24 > ER_VERS_TEMPORARY > eng "TEMPORARY tables do not support system versioning" > > -ER_VERS_NOT_SUPPORTED > - eng "%s is not supported for %s system-versioned tables" > +ER_VERS_TRX_PART_HISTORIC_ROW_NOT_SUPPORTED > + eng "Transactional system versioned tables do not support partitioning by ROW START or ROW END" > ER_INDEX_FILE_FULL > eng "The index file for table '%-.192s' is full" Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 842ef80b029: MDEV-18464: Port kill_one_trx fixes from 10.4 to 10.1
by Marko Mäkelä 26 Mar '19

26 Mar '19

Hi Jan, On Fri, Mar 22, 2019 at 1:59 PM jan <jan.lindstrom(a)mariadb.com> wrote: > > revision-id: 842ef80b029c975b7f7f7ec25fe55817ec626f02 (mariadb-10.1.38-68-g842ef80b029) Was this pushed to buildbot as well? I did not find the commit ID. First, some general comments: * The functions trx_start_if_not_started_low() and trx_start_for_ddl_low() differ between XtraDB and InnoDB with regard to #ifdef WITH_WSREP. Why? Are the debug assignments truly necessary? I see that the difference was introduced 5 years ago in MDEV-6247. * The files storage/{innobase,xtradb}/trx/trx0roll.cc differ from each other after the change. It looks like the innobase version is better. Please remove the unnecessary lines from xtradb: --- storage/xtradb/trx/trx0roll.cc 2019-03-26 10:20:31.767971537 +0200 +++ storage/innobase/trx/trx0roll.cc 2019-03-26 10:20:31.759971465 +0200 @@ -33,8 +33,6 @@ #include "trx0roll.ic" #endif -#include <mysql/service_wsrep.h> - #include "fsp0fsp.h" #include "mach0data.h" #include "trx0rseg.h" @@ -51,9 +49,6 @@ #include "pars0pars.h" #include "srv0mon.h" #include "trx0sys.h" -#ifdef WITH_WSREP -#include "ha_prototypes.h" -#endif /* WITH_WSREP */ /** This many pages must be undone before a truncate is tried within rollback */ @@ -1075,12 +1070,6 @@ if (trx->update_undo) { trx_undo_truncate_end(trx, trx->update_undo, limit); } - -#ifdef WITH_WSREP_OUT - if (wsrep_on(trx->mysql_thd)) { - trx->lock.was_chosen_as_deadlock_victim = FALSE; - } -#endif /* WITH_WSREP */ } /***********************************************************************//** On to the detailed review: > diff --git a/storage/innobase/include/trx0trx.h b/storage/innobase/include/trx0trx.h > index fe16b8272b8..b83734b154a 100644 > --- a/storage/innobase/include/trx0trx.h > +++ b/storage/innobase/include/trx0trx.h > @@ -1,7 +1,7 @@ > /***************************************************************************** > > Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. > -Copyright (c) 2015, 2018, MariaDB Corporation. > +Copyright (c) 2015, 2019, MariaDB Corporation. > > This program is free software; you can redistribute it and/or modify it under > the terms of the GNU General Public License as published by the Free Software > @@ -613,6 +613,9 @@ struct trx_lock_t { > transaction as a victim in deadlock > resolution, it sets this to TRUE. > Protected by trx->mutex. */ > + bool was_chosen_as_replication_victim; > + /* replication has marked this > + trx to abort */ > time_t wait_started; /*!< lock wait started at this time, > protected only by lock_sys->mutex */ > > @@ -624,6 +627,12 @@ struct trx_lock_t { > only be modified by the thread that is > serving the running transaction. */ > > +#ifdef WITH_WSREP > + bool was_chosen_as_wsrep_victim; > + /*!< high priority wsrep thread has > + marked this trx to abort */ > +#endif /* WITH_WSREP */ > + > mem_heap_t* lock_heap; /*!< memory heap for trx_locks; > protected by lock_sys->mutex */ > > @@ -695,14 +704,6 @@ lock_rec_convert_impl_to_expl()) will access transactions associated > to other connections. The locks of transactions are protected by > lock_sys->mutex and sometimes by trx->mutex. */ > > -enum trx_abort_t { > - TRX_SERVER_ABORT = 0, > -#ifdef WITH_WSREP > - TRX_WSREP_ABORT, > -#endif > - TRX_REPLICATION_ABORT > -}; > - > struct trx_t{ > ulint magic_n; > > @@ -880,8 +881,6 @@ struct trx_t{ > /*------------------------------*/ > THD* mysql_thd; /*!< MySQL thread handle corresponding > to this trx, or NULL */ > - trx_abort_t abort_type; /*!< Transaction abort type*/ > - > const char* mysql_log_file_name; > /*!< if MySQL binlog is used, this field > contains a pointer to the latest file Why are you replacing trx_t::abort_type with two fields in trx_lock_t that are located far apart from each other? To avoid changing the performance too much due to caching, I would suggest to introduce the two fields in trx_t, replacing the former location of abort_type. Do we really need such long names? Would trx_t::replication_victim and trx_t::wsrep_victim suffice? Please document how concurrent access to the fields is being protected. Apparently for the WSREP field it is both trx->mutex and lock_sys->mutex. > diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc > index 3970a559a4c..984ac6cacbc 100644 > --- a/storage/innobase/lock/lock0lock.cc > +++ b/storage/innobase/lock/lock0lock.cc > @@ -4785,9 +4783,8 @@ lock_report_waiters_to_mysql( > innobase_kill_query. We mark this by setting > current_lock_mutex_owner, so we can avoid trying > to recursively take lock_sys->mutex. */ > - w_trx->abort_type = TRX_REPLICATION_ABORT; > + w_trx->lock.was_chosen_as_replication_victim = true; > thd_report_wait_for(mysql_thd, w_trx->mysql_thd); > - w_trx->abort_type = TRX_SERVER_ABORT; > } > ++i; > } Here, we seem to be only holding lock_sys->mutex, not w_trx->mutex. > @@ -7993,6 +7981,37 @@ lock_trx_handle_wait( > return DB_LOCK_WAIT; > } > > +/*********************************************************************//** > +Check whether the transaction has already been rolled back because it > +was selected as a deadlock victim, or if it has to wait then cancel > +the wait lock. > +@return DB_DEADLOCK, DB_LOCK_WAIT or DB_SUCCESS */ > +UNIV_INTERN > +dberr_t > +lock_trx_handle_wait( > +/*=================*/ > + trx_t* trx) /*!< in/out: trx lock state */ > +{ > + bool lock_mutex_taken = false; Do we really need this variable? > +#ifdef WITH_WSREP > + /* We already own mutexes */ > + if (trx->lock.was_chosen_as_wsrep_victim) { > + return lock_trx_handle_wait_low(trx); > + } > +#endif /* WITH_WSREP */ > + if (!trx->lock.was_chosen_as_replication_victim) { > + lock_mutex_enter(); > + lock_mutex_taken = true; > + } > + trx_mutex_enter(trx); > + dberr_t err = lock_trx_handle_wait_low(trx); > + if (lock_mutex_taken) { > + lock_mutex_exit(); > + } > + trx_mutex_exit(trx); > + return err; > +} I would expand the second part of the code and remove the condition before releasing the 2 mutexes: if (!trx->lock.was_chosen_as_replication_victim) { lock_mutex_enter(); trx_mutex_enter(trx); dberr_t err = lock_trx_handle_wait_low(trx); lock_mutex_exit(); trx_mutex_exit(trx); return err; } else { trx_mutex_enter(trx); dberr_t err = lock_trx_handle_wait_low(trx); trx_mutex_exit(trx); return err; } Also, I think that it would be better to declare lock_trx_handle_wait_low() only static but not inline, to reduce unnecessary code expansion. Best regards, Marko

1 0

Re: [Maria-developers] 0fa832dc426: Revert THD::THD(skip_global_sys_var_lock) argument
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 0fa832dc426 (mariadb-10.3.12-97-g0fa832dc426) > parent(s): 6f14489ad31 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-21 17:17:26 +0400 > message: > > Revert THD::THD(skip_global_sys_var_lock) argument > > Originally introduced by e972125f1 to avoid harmless wait for > LOCK_global_system_variables in a newly created thread, which creation was > initiated by system variable update. > > At the same time it opens dangerous hole, when system variable update > thread already released LOCK_global_system_variables and ack_receiver > thread haven't yet completed new THD construction. In this case THD > constructor goes completely unprotected. > > Since ack_receiver.stop() waits for the thread to go down, we have to > temporarily release LOCK_global_system_variables so that it doesn't > deadlock with ack_receiver.run(). Unfortunately it breaks atomicity > of rpl_semi_sync_master_enabled updates and makes them not serialized. > > LOCK_rpl_semi_sync_master_enabled was introduced to workaround the above. > TODO: move ack_receiver start/stop into repl_semisync_master > enable_master/disable_master under LOCK_binlog protection? > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 6f14489ad31: Adieu find_sys_var_ex()
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 6f14489ad31 (mariadb-10.3.12-96-g6f14489ad31) > parent(s): 48af6851da2 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-21 01:32:04 +0400 > message: > > Adieu find_sys_var_ex() > > Only take LOCK_plugin for plugin system variables. > > Reverted optimisation that was originally done for session tracker: it > makes much less sense now. Specifically only if connections would want to > track plugin session variables changes and these changes would actually > happen frequently. If this ever becomes an issue, there're much better > ways to optimise this workload. > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 48af6851da2: Make connect speed great again
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! On Mar 25, Sergey Vojtovich wrote: > revision-id: 48af6851da2 (mariadb-10.3.12-95-g48af6851da2) > parent(s): a83d6ee812b > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-20 01:58:42 +0400 > message: > > Make connect speed great again > > Rather than parsing session_track_system_variables when thread starts, do > it when first trackable event occurs. > > Benchmarked on a 2socket/20core/40threads Broadwell system using sysbench > connect brencmark @40 threads (with select 1 disabled): > 101379.77 -> 143016.68 CPS, whereas 10.2 is currently at 137766.31 CPS. > > Part of MDEV-14984 - regression in connect performance > > diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc > index 5d98e58046d..c493c790596 100644 > --- a/sql/session_tracker.cc > +++ b/sql/session_tracker.cc > @@ -378,16 +378,9 @@ bool Session_sysvars_tracker::configure() > > bool Session_sysvars_tracker::enable(THD *thd) > { > - LEX_STRING tmp= { session_track_system_variables, > - safe_strlen(session_track_system_variables) }; > orig_list.reinit(); > - if (orig_list.parse_var_list(thd, tmp, true, thd->charset()) == true) > - { > - orig_list.reinit(); > - m_enabled= false; > - return true; > - } > - m_enabled= true; > + m_parsed= false; > + m_enabled= session_track_system_variables && *session_track_system_variables; wouldn't it be simpler not to allow both NULL and "" as valid values for session_track_system_variables ? Either always store the empty string as NULL or as "", why would you want both? > return false; > } > > @@ -530,6 +524,19 @@ void Session_sysvars_tracker::mark_as_changed(THD *thd, > { > sysvar_node_st *node; > sys_var *svar= (sys_var *)var; > + > + if (!m_parsed) > + { > + LEX_STRING tmp= { session_track_system_variables, > + safe_strlen(session_track_system_variables) }; > + if (orig_list.parse_var_list(thd, tmp, true, thd->charset())) > + { > + orig_list.reinit(); > + return; > + } > + m_parsed= true; This is of somewhat dubious value. The default value for the @@session_track_system_variables is DEFAULT("autocommit,character_set_client,character_set_connection," "character_set_results,time_zone"), and these are variables that are typically set by connectors automatically on every new connection. Only a pure C/C will benefit from it, that's what sysbench shows. If you'd tested with java, you, probably, wouldn't get any speedup. > + } > + > /* > Check if the specified system variable is being tracked, if so > mark it as changed and also set the class's m_changed flag. > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 7194bbd03ef..53203d15f0b 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -7320,8 +7320,13 @@ void THD::set_last_commit_gtid(rpl_gtid &gtid) > #ifndef EMBEDDED_LIBRARY > if (changed_gtid && session_tracker.session_sysvars_tracker.is_enabled()) > { > + THD *orig_thd= current_thd; > + if (orig_thd != this) When can it be true? > + set_current_thd(this); > session_tracker.session_sysvars_tracker. > mark_as_changed(this, (LEX_CSTRING*)Sys_last_gtid_ptr); > + if (orig_thd != this) > + set_current_thd(orig_thd); > } > #endif > } > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] a83d6ee812b: Safe session_track_system_variables snapshot
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! Just one question. Why do you need Session_sysvars_tracker::session_track_system_variables if you have THD::system_variables::session_track_system_variables ? On Mar 25, Sergey Vojtovich wrote: > revision-id: a83d6ee812b (mariadb-10.3.12-94-ga83d6ee812b) > parent(s): 3c6f17a130b > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 23:40:08 +0400 > message: > > Safe session_track_system_variables snapshot > > When enabling system variables tracker, make a copy of > global_system_variables.session_track_system_variables > under LOCK_global_system_variables. This protects from concurrent variable > updates and potential freed memory access, as well as from variable > reconstruction (which was previously protected by LOCK_plugin). > > We can also use this copy as a session variable pointer, so that we don't > have to allocate memory and reconstruct it every time it is referenced. > > For this very reason we don't need buffer_length stuff anymore. > > As well as don't we need to take LOCK_plugin early in ::enable(). > Unified ::parse_var_list() to acquire LOCK_plugin unconditionally. > For no apparent reason it wasn't previously acquired for global > variable update. > > No reason to access session_sysvars_tracker via get_tracker(), so access > it directly instead. > > Part of MDEV-14984 - regression in connect performance Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 3c6f17a130b: Allocate Session_sysvars_tracker statically
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! Please, see few questions below On Mar 25, Sergey Vojtovich wrote: > revision-id: 3c6f17a130b (mariadb-10.3.12-93-g3c6f17a130b) > parent(s): db9854b7d48 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 23:40:07 +0400 > message: > > Allocate Session_sysvars_tracker statically > > One less new/delete per connection. > > Removed m_mem_flag since most allocs are thread specific. The only > exception are allocs performed during initialization. > > Removed State_tracker and Session_tracker constructors as they don't make > sense anymore. > > Rather than having session_tracker.deinit(), fixed memory accounting so > that it gets initialized before any other constructor and deinitialized > after all destructors done. Cons: makes THD a few pointers larger :( > > Part of MDEV-14984 - regression in connect performance > > diff --git a/sql/sql_class.h b/sql/sql_class.h > index 69e6f58f854..64d772e5d17 100644 > --- a/sql/sql_class.h > +++ b/sql/sql_class.h > @@ -2145,13 +2145,34 @@ struct wait_for_commit > > extern "C" void my_message_sql(uint error, const char *str, myf MyFlags); > > + > +/** > + A wrapper around thread_count. > + > + It must be specified as a first base class of THD, so that increment is > + done before any other THD constructors and decrement - after any other THD > + destructors. > + > + Destructor unblocks close_conneciton() if there are no more THD's left. > +*/ > +class THD_count > +{ > + THD *orig_thd; > + THD_count(); > + ~THD_count(); > + friend class THD; > +}; 1. Why a separate class, what's wrong with having it in THD? No downcasts, no risk that someone would try to use it separately. 2. orig_thd looks very suspicious. You preserve the old THD* pointer for the whole duration of this THD lifetime and restore it at the end. How can you know that the "orig_thd" is still valid? Why do you need to do it anyway? > + > + > /** > @class THD > For each client connection we create a separate thread with THD serving as > a thread/connection descriptor > */ > > -class THD :public Statement, > +class THD: public THD_count, /* this must be first */ an assert would be better than "/* must be first */". Like, assert(&orig_thd == this) or something. > + public Statement, > + > /* > This is to track items changed during execution of a prepared > statement/stored procedure. It's created by > @@ -2176,19 +2197,6 @@ class THD :public Statement, > inline bool is_conventional() const > { DBUG_ASSERT(0); return Statement::is_conventional(); } > > - void dec_thread_count(void) > - { > - DBUG_ASSERT(thread_count > 0); > - thread_safe_decrement32(&thread_count); > - signal_thd_deleted(); > - } > - > - > - void inc_thread_count(void) > - { > - thread_safe_increment32(&thread_count); > - } > - > public: > MDL_context mdl_context; > > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 9c9fa228e2a..afe4123a52c 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -597,8 +597,46 @@ extern "C" void thd_kill_timeout(THD* thd) > thd->awake(KILL_TIMEOUT); > } > > -THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock) > - :Statement(&main_lex, &main_mem_root, STMT_CONVENTIONAL_EXECUTION, > + > +THD_count::THD_count() > +{ > + THD *thd= static_cast<THD*>(this); > + thread_safe_increment32(&thread_count); > + /* > + We set THR_THD to temporally point to this THD to register all the > + variables that allocates memory for this THD > + */ > + orig_thd= current_thd; > + set_current_thd(thd); > + thd->status_var.local_memory_used= sizeof(THD); > + thd->status_var.max_local_memory_used= thd->status_var.local_memory_used; > + thd->status_var.global_memory_used= 0; > + thd->variables.max_mem_used= global_system_variables.max_mem_used; > +} > + > + > +THD_count::~THD_count() > +{ > + THD *thd= static_cast<THD*>(this); > + if (thd->status_var.local_memory_used != 0) > + { > + DBUG_PRINT("error", ("memory_used: %lld", > + thd->status_var.local_memory_used)); > + SAFEMALLOC_REPORT_MEMORY(thd->thread_id); > + DBUG_ASSERT(thd->status_var.local_memory_used == 0 || > + !debug_assert_on_not_freed_memory); > + } > + update_global_memory_status(thd->status_var.global_memory_used); > + set_current_thd(orig_thd); > + DBUG_ASSERT(thread_count > 0); > + thread_safe_decrement32(&thread_count); > + signal_thd_deleted(); > +} > + > + > +THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock): > + THD_count(), > + Statement(&main_lex, &main_mem_root, STMT_CONVENTIONAL_EXECUTION, > /* statement id */ 0), > rli_fake(0), rgi_fake(0), rgi_slave(NULL), > protocol_text(this), protocol_binary(this), > @@ -654,15 +692,6 @@ THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock) > ulong tmp; > bzero(&variables, sizeof(variables)); > > - /* > - We set THR_THD to temporally point to this THD to register all the > - variables that allocates memory for this THD > - */ > - THD *old_THR_THD= current_thd; > - set_current_thd(this); > - status_var.local_memory_used= sizeof(THD); > - status_var.max_local_memory_used= status_var.local_memory_used; > - status_var.global_memory_used= 0; > variables.pseudo_thread_id= thread_id; > variables.max_mem_used= global_system_variables.max_mem_used; > main_da.init(); > @@ -849,8 +878,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier, bool skip_global_sys_var_lock) > save_prep_leaf_list= FALSE; > org_charset= 0; > /* Restore THR_THD */ > - set_current_thd(old_THR_THD); > - inc_thread_count(); > + set_current_thd(orig_thd); > } > > > @@ -1589,7 +1617,6 @@ void THD::reset_for_reuse() > > THD::~THD() > { > - THD *orig_thd= current_thd; > THD_CHECK_SENTRY(this); > DBUG_ENTER("~THD()"); > /* Check that we have already called thd->unlink() */ > @@ -1601,7 +1628,11 @@ THD::~THD() > In error cases, thd may not be current thd. We have to fix this so > that memory allocation counting is done correctly > */ > - set_current_thd(this); > + orig_thd= current_thd; > + if (orig_thd == this) > + orig_thd= 0; > + else > + set_current_thd(this); > if (!status_in_global) > add_status_to_global(); > > @@ -1655,22 +1686,6 @@ THD::~THD() > lf_hash_put_pins(xid_hash_pins); > /* Ensure everything is freed */ > status_var.local_memory_used-= sizeof(THD); > - > - /* trick to make happy memory accounting system */ > -#ifndef EMBEDDED_LIBRARY > - session_tracker.deinit(); > -#endif //EMBEDDED_LIBRARY > - > - if (status_var.local_memory_used != 0) > - { > - DBUG_PRINT("error", ("memory_used: %lld", status_var.local_memory_used)); > - SAFEMALLOC_REPORT_MEMORY(thread_id); > - DBUG_ASSERT(status_var.local_memory_used == 0 || > - !debug_assert_on_not_freed_memory); > - } > - update_global_memory_status(status_var.global_memory_used); > - set_current_thd(orig_thd == this ? 0 : orig_thd); > - dec_thread_count(); > DBUG_VOID_RETURN; > } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 72e4e57a52a: Cleanup session tracker API
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 72e4e57a52a (mariadb-10.3.12-88-g72e4e57a52a) > parent(s): 68abb07298b > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 17:11:16 +0400 > message: > > Cleanup session tracker API > > - Session_sysvars_tracker::server_init_check() -> sysvartrack_validate_value() > - Session_sysvars_tracker::check_var_list() -> sysvartrack_validate_value() > > - Session_sysvars_tracker::server_init_process() -> sysvartrack_global_update() > - sysvartrack_reprint_value() -> sysvartrack_global_update() > > - sysvartrack_value_len() -> sysvartrack_session_value_ptr() > - sysvartrack_value_construct() -> sysvartrack_session_value_ptr() > > - sysvartrack_update() -> Session_sysvars_tracker::update() > - Session_tracker::server_boot_verify() -> session_tracker_init() > - sysvar_tracker() -> /dev/null > > Part of MDEV-14984 - regression in connect performance Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 62d62228aa6: Static current schema and state change trackers
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! Why two trackers are static and the other two aren't? Please, explain it in a comment. On Mar 25, Sergey Vojtovich wrote: > revision-id: 62d62228aa6 (mariadb-10.3.12-91-g62d62228aa6) > parent(s): 61b9bffbedc > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 23:40:07 +0400 > message: > > Static current schema and state change trackers > > Saves 2 new/delete per connection. > > Part of MDEV-14984 - regression in connect performance Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 1

Re: [Maria-developers] db9854b7d48: Allocate Transaction_state_tracker statically
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: db9854b7d48 (mariadb-10.3.12-92-gdb9854b7d48) > parent(s): 62d62228aa6 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 23:40:07 +0400 > message: > > Allocate Transaction_state_tracker statically > > One less new/delete per connection. > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 61b9bffbedc: Allocate orig_list statically
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 61b9bffbedc (mariadb-10.3.12-90-g61b9bffbedc) > parent(s): 9dd4fcb3f7c > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 23:39:19 +0400 > message: > > Allocate orig_list statically > > tool_list is a temporary list needed only for SET SESSION > session_track_system_variables, so allocate it on stack instead. > > Sane reinit() calls. > > Saves 2 new/delete per connection. > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 9dd4fcb3f7c: Get rid of not implemented SESSION_GTIDS_TRACKER
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 9dd4fcb3f7c (mariadb-10.3.12-89-g9dd4fcb3f7c) > parent(s): 72e4e57a52a > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-19 17:28:09 +0400 > message: > > Get rid of not implemented SESSION_GTIDS_TRACKER > > One less new/delete per connection. > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 4c0eb412ff5: A proper State_tracker::m_changed enacpsulation
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 4c0eb412ff5 (mariadb-10.3.12-85-g4c0eb412ff5) > parent(s): 69b33fca8cc > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-18 22:51:45 +0400 > message: > > A proper State_tracker::m_changed enacpsulation > > Note: preserved original behaviour, where remaining trackers are not reset > on error from store(). This effectively means subsequent statements will > start tracking from unclean state. > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 68abb07298b: Cleanup session tracker redundnacy
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! ok to push On Mar 25, Sergey Vojtovich wrote: > revision-id: 68abb07298b (mariadb-10.3.12-87-g68abb07298b) > parent(s): facc973189a > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-18 22:51:58 +0400 > message: > > Cleanup session tracker redundnacy > > - m_enabled is initialised by the base class constructor > - removed unused schema_track_inited > - moved Transaction_state_tracker constructor to declaration > - common enable() > - removed unused Session_sysvars_tracker::check_str() > - classes are "private" by default > - don't even try to compile for embedded > > Part of MDEV-14984 - regression in connect performance > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] facc973189a: Session_sysvars_tracker::vars_list cleanups
by Sergei Golubchik 25 Mar '19

25 Mar '19

Hi, Sergey! On Mar 25, Sergey Vojtovich wrote: > revision-id: facc973189a (mariadb-10.3.12-86-gfacc973189a) > parent(s): 4c0eb412ff5 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-03-18 22:51:58 +0400 > message: > > Session_sysvars_tracker::vars_list cleanups > > - return proper type > - removed useless node argument > - removed useless mem_flag > - classes are "private" by default > - simplified iterators > > Part of MDEV-14984 - regression in connect performance ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MDEV-18734 ASAN heap-use-after-free in my_strnxfrm_simple_internal upon update on versioned partitioned table
by Aleksey Midenkov 25 Mar '19

25 Mar '19

Hi, Sergei! ha_partition::handle_ordered_index_scan() stores records in m_ordered_rec_buffer. Then TABLE::update_virtual_fields() updates blob buffer and frees the old one. Then ha_partition::return_top_record() returns record from m_ordered_rec_buffer with stale blob pointer. What should we do with this? I propose to duplicate blob buffer when record gets into m_ordered_rec_buffer. More details can be found here: https://github.com/tempesta-tech/mariadb/issues/581 -- All the best, Aleksey Midenkov @midenok

2 4

[Maria-developers] question about running latest build.
by Luuk 24 Mar '19

24 Mar '19

After noticing that a bug i reported was set to 'solved' i wanted to see/check that. So, i downloaded/compiled Now i seem to be able to run the server, but i am unable to connect using 'mysql.exe' Luuk@ZES MINGW64 /d/TEMP/server (master) $ bld/client/RelWithDebInfo/mysqladmin -h localhost -u root version D:\TEMP\server\bld\client\RelWithDebInfo\mysqladmin.exe Ver 9.1 Distrib 10.4.4-MariaDB, for Win32 on AMD64 Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others. Server version 10.4.4-MariaDB Protocol version 10 Connection localhost via TCP/IP TCP port 3306 Uptime: 23 min 15 sec Threads: 7 Questions: 4 Slow queries: 0 Opens: 16 Flush tables: 1 Open tables: 10 Queries per second avg: 0.002 Luuk@ZES MINGW64 /d/TEMP/server (master) $ bld/client/RelWithDebInfo/mysql -h localhost -u root #### no reaction, killin 'mysql.exe' just add another 'Aborted connection' to the server-log.... QUESTION: What am i missing here? The log from the server (Windows 10): Luuk@ZES MINGW64 /d/TEMP/server/bld (master) $ sql/RelWithDebInfo/mysqld.exe --console 2019-03-24 16:20:01 0 [Note] D:\TEMP\server\bld\sql\RelWithDebInfo\mysqld.exe (mysqld 10.4.4-MariaDB) starting as process 2024 ... 2019-03-24 16:20:01 0 [Note] InnoDB: Mutexes and rw_locks use Windows interlocked functions 2019-03-24 16:20:01 0 [Note] InnoDB: Uses event mutexes 2019-03-24 16:20:01 0 [Note] InnoDB: Compressed tables use zlib 1.2.11 2019-03-24 16:20:01 0 [Note] InnoDB: Number of pools: 1 2019-03-24 16:20:01 0 [Note] InnoDB: Using SSE2 crc32 instructions 2019-03-24 16:20:01 0 [Note] InnoDB: Initializing buffer pool, total size = 128M, instances = 1, chunk size = 128M 2019-03-24 16:20:01 0 [Note] InnoDB: Completed initialization of buffer pool 2019-03-24 16:20:01 0 [Note] InnoDB: 128 out of 128 rollback segments are active. 2019-03-24 16:20:01 0 [Note] InnoDB: Creating shared tablespace for temporary tables 2019-03-24 16:20:01 0 [Note] InnoDB: Setting file '.\ibtmp1' size to 12 MB. Physically writing the file full; Please wait ... 2019-03-24 16:20:01 0 [Note] InnoDB: File '.\ibtmp1' size is now 12 MB. 2019-03-24 16:20:01 0 [Note] InnoDB: Waiting for purge to start 2019-03-24 16:20:01 0 [Note] InnoDB: 10.4.4 started; log sequence number 139881; transaction id 21 2019-03-24 16:20:01 0 [Note] InnoDB: Loading buffer pool(s) from D:\TEMP\server\bld\sql\data\ib_buffer_pool 2019-03-24 16:20:01 0 [Note] Plugin 'FEEDBACK' is disabled. 2019-03-24 16:20:01 0 [Note] InnoDB: Buffer pool(s) load completed at 190324 16:20:01 2019-03-24 16:20:01 0 [Note] Server socket created on IP: '::'. 2019-03-24 16:20:01 0 [Note] Reading of all Master_info entries succeded 2019-03-24 16:20:01 0 [Note] Added new Master_info '' to hash table 2019-03-24 16:20:01 0 [Note] D:\TEMP\server\bld\sql\RelWithDebInfo\mysqld.exe: ready for connections. Version: '10.4.4-MariaDB' socket: '' port: 3306 Source distribution 2019-03-24 16:21:24 8 [Warning] Aborted connection 8 to db: 'unconnected' user: 'root' host: 'localhost' (Got an error reading communication packets) 2019-03-24 16:21:49 9 [Warning] Aborted connection 9 to db: 'unconnected' user: 'root' host: 'localhost' (Got an error reading communication packets)

1 0

Re: [Maria-developers] d260871fccc: MDEV-18967 Load data in system version with long unique does not work
by Sergei Golubchik 20 Mar '19

20 Mar '19

Hi, Sachin! ok to push! On Mar 20, Sachin Setiya wrote: > revision-id: d260871fccc (mariadb-10.4.3-87-gd260871fccc) > parent(s): f6166101e0f > author: Sachin <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-19 16:43:43 +0530 > message: > > MDEV-18967 Load data in system version with long unique does not work > > Update system versioning fields before generaled columns for left out > fill_record > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] f6166101e0f: MDEV-18904 Assertion `m_part_spec.start_part >= m_part_spec.end_part' failed in ha_partition::index_read_idx_map
by Sergei Golubchik 20 Mar '19

20 Mar '19

Hi, Sachin! On Mar 20, Sachin Setiya wrote: > revision-id: f6166101e0f (mariadb-10.4.3-86-gf6166101e0f) > parent(s): 8076c594ce2 > author: sachinsetia1001(a)gmail.com <sachin.setiya(a)mariadb.com> > committer: Sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-19 16:43:43 +0530 > message: > > MDEV-18904 Assertion `m_part_spec.start_part >= m_part_spec.end_part' failed in ha_partition::index_read_idx_map > > We do not support long unique index in partition. Why not? I would think that the assert is wrong, there is no reason why ha_partition::index_read_idx_map() would always hit just one partition. ha_partition::index_read_map() doesn't have this assert. And if you check the history of the assert you'll find that the comment originally was /* How can it be more than one partition with the current use? */ So, apparently now you've introduced a new "use" and there can be more than one partition, and removing the assert looks justified. > +CREATE TABLE t1 (a int , unique(a) using hash) PARTITION BY HASH (a) PARTITIONS 2; > +ERROR HY000: Long unique index is not supported in partition Also - for the future - there are no "long unique" indexes from the user point of view. This is only an internal name. The SQL syntax does use "long unique" concept, documentation doesn't, error messages don't. That is, users simply won't understand what you mean here. If we'll need an error here, I'd suggest, for example, HASH UNIQUE indexes cannot be used in partitioned tables. but it's not strictly correct, if some engine would implement hash indexes natively. The more precise message could be HASH UNIQUE indexes cannot be used in par titioned MyISAM tables. or Partitioned MyISAM tables do not support HASH UNIQUE indexes. (where "MyISAM" is a parameter, of course). But, again, I am not yet convinced that we need this limitation at all. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 500d909: MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly
by Sergey Petrunia 18 Mar '19

18 Mar '19

> revision-id: 500d909a795022255fb6f6c44cd310636afc0b5a (mariadb-10.3.6-226-g500d909) > parent(s): 88d89ee0bae24b71416c2af4f4c2f2be7b6a033a > author: Varun Gupta > committer: Varun Gupta > timestamp: 2019-03-11 20:20:35 +0530 > message: > > MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly > > Keys with multi-key parts were not being printed correctly, only the first key part > was getting printed. > Fixed it by making sure append_range_all_keyparts function is called for the remaining keyparts. > ... > diff --git a/mysql-test/main/opt_trace.result b/mysql-test/main/opt_trace.result > index 4c3e2b3..a4c2b86 100644 > --- a/mysql-test/main/opt_trace.result > +++ b/mysql-test/main/opt_trace.result > @@ -6022,3 +6022,428 @@ t_outer_2.a in (select t_inner_3.a from t2 t_inner_3, t1 t_inner_4) { > set @@optimizer_switch= @save_optimizer_switch; > drop table t1,t2; > set optimizer_trace='enabled=off'; > +# > +# MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly > +# > +create table t0(a int); > +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); > +create table one_k (a int); > +insert into one_k select A.a + B.a*10 + C.a*100 from t0 A, t0 B, t0 C; > +create table t1 ( a int, b int, c int, d int, key (a,b), key(c,d)); > +insert into t1 select a,a,a,a from one_k; > +set optimizer_trace=1; > +explain format=json select * from t1 where a > 10 and b < 10 and c=0 and d=1; ... > +select * from INFORMATION_SCHEMA.OPTIMIZER_TRACe; This produces a lot of irrelevant output. Can you use something like select JSON_DETAILED(JSON_EXTRACT(a, '$**.range_analysis')) from optimizer_trace to select on the part of the trace that we need? > + "analyzing_range_alternatives": { > + "range_scan_alternatives": [ > + { > + "index": "a", > + "ranges": ["10 < a AND NULL < b < 10"], This is not a valid range. If one forces this quick select to be picked for the query plan, they can look into .trace and see the ranges: explain format=json select * from t1 force index (a) where a > 10 and b < 10; T@19 : | | | | | | | | | | >print_quick quick range select, key a, length: 5 10 < X other_keys: 0x0: T@19 : | | | | | | | | | | <print_quick (you can also try with a >=10 to see a different kind of range being produced). (One could argue that what is printed is a representation of the SEL_ARG graph, but in that case it is extremely confusing. I think, we should start with showing ranges, not SEL_ARG graphs). I think, we should not try to intrepret the SEL_ARG graph ourselves but rather use sel_arg_range_seq_init / sel_arg_range_seq_next to produce ranges that can be printed. Any objections to this? > + "rowid_ordered": false, > + "using_mrr": false, > + "index_only": false, > + "rows": 989, > + "cost": 1272.8, > + "chosen": false, > + "cause": "cost" > + }, > + { > + "index": "c", > + "ranges": ["0 <= c <= 0 AND 1 <= d <= 1"], > + "rowid_ordered": true, > + "using_mrr": false, > + "index_only": false, > + "rows": 1, > + "cost": 2.3783, > + "chosen": true > + } > + ], BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] ASAN segfaults on vcol case
by Aleksey Midenkov 18 Mar '19

18 Mar '19

This case segfaults in ASAN code: delimiter ~~ create or replace procedure t1() begin create or replace table t1 ( b tinytext, v text as (b) virtual ); end~~ delimiter ; call t1; Both in 10.3 and 10.4 in different places. Does someone know about such ASAN faults? -- All the best, Aleksey Midenkov @midenok

2 2

Re: [Maria-developers] 21a15ea673c: MDEV-16240: Assertion `0' failed in row_sel_convert_mysql_key_to_innobase
by Sergei Golubchik 18 Mar '19

18 Mar '19

Hi, Oleksandr! On Mar 18, Oleksandr Byelkin wrote: > revision-id: 21a15ea673c (mariadb-10.2.22-72-g21a15ea673c) > parent(s): bb8c82c66ab > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2019-03-14 15:12:47 +0100 > message: > > MDEV-16240: Assertion `0' failed in row_sel_convert_mysql_key_to_innobase > > Set table in row ID position mode before using this function. > > diff --git a/mysql-test/r/multi_update_innodb.result b/mysql-test/r/multi_update_innodb.result > index 5890fd24f5f..294ebfcebdf 100644 > --- a/mysql-test/r/multi_update_innodb.result > +++ b/mysql-test/r/multi_update_innodb.result > @@ -151,3 +151,43 @@ create table t2 like t1; > insert into t2 select * from t1; > delete t1,t2 from t2,t1 where t1.a<'B' and t2.b=t1.b; > drop table t1,t2; > +# > +# MDEV-16240: Assertion `0' failed in > +# row_sel_convert_mysql_key_to_innobase > +# > +SET @save_sql_mode=@@sql_mode; > +set sql_mode=''; > +CREATE TABLE `t3` ( > +`f1` TIMESTAMP NOT NULL DEFAULT '0000-00-00 00:00:00' ON UPDATE current_timestamp(), > +`f2` datetime DEFAULT '2000-01-01 00:00:00' ON UPDATE current_timestamp(), > +`f3` TIMESTAMP NULL DEFAULT '2000-01-01 00:00:00', > +`pk` datetime NOT NULL DEFAULT '0000-00-00 00:00:00', > +`f4` datetime DEFAULT current_timestamp(), > +PRIMARY KEY (`pk`), > +UNIQUE KEY `f2k` (`f2`), > +KEY `f4k` (`f4`) > +) ENGINE=InnoDB; > +INSERT INTO `t3` VALUES ('2018-05-18 15:08:07','2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00','0000-00-00 00:00:00'),('0000-00-00 00:00:00','0000-00-00 00:00:00','1999-12-31 23:00:00','2002-07-03 23:04:40','0000-00-00 00:00:00'); > +CREATE VIEW `v1` AS > +SELECT `t3`.`pk` AS `pk`, > +`t3`.`f3` AS `f3`, > +`t3`.`f4` AS `f4`, > +`t3`.`f2` AS `f2`, > +`t3`.`f1` AS `f1` > +FROM `t3`; > +CREATE TABLE `t4` ( > +`f1` datetime DEFAULT current_timestamp() ON UPDATE current_timestamp(), > +`f3` timestamp NULL DEFAULT NULL, > +`f2` timestamp NULL DEFAULT '1999-12-31 23:00:00' ON UPDATE current_timestamp(), > +`pk` int(11) NOT NULL, > +`f4` timestamp NOT NULL DEFAULT current_timestamp() ON UPDATE current_timestamp(), > +PRIMARY KEY (`pk`) > +) ENGINE=InnoDB; > +INSERT INTO `t4` VALUES ('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,1,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,3,'2018-05-18 15:08:06'),('0000-00-00 00:00:00','0000-00-00 00:00:00',NULL,1976,'0000-00-00 00:00:00'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2000,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2001,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2002,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2003,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00',NULL,2004,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2005,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2018,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2019,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','2018-05-18 15:08:06',2024,'2018-05-18 15:08:06'),('2018-05-18 17:08:06','0000-00-00 00:00:00','1999-12-31 23:00:00',2025,'2018-05-18 15:08:06'),('0000-00-00 00:00:00',NULL,'2018-05-18 15:08:06',2026,'2018-05-18 15:08:06'),('2018-05-18 17:08:07','0000-00-00 00:00:00','0000-00-00 00:00:00',2027,'0000-00-00 00:00:00'); > +UPDATE `v1` t1, `t4` t2 > +SET t1.`f2` = 6452736 WHERE t1.`f4` = 6272000; > +ERROR 23000: Duplicate entry '0000-00-00 00:00:00' for key 'f2k' actually, when I wrote "please, rename all columns in the test to have unique names" I meant _all_ columns, that is, in the t3 columnd could've been f1-f5, in v1 they could be called f6-f10, in t4 - f11-f15. but don't bother renaming them now, apparently the most important name was 'f2k' and it's unique now, so it's sufficiently clear, thanks. Still, for the future, it's generally helpful to have all names different to be able to understand what the error message refers to. > +DROP VIEW v1; > +DROP TABLE t3,t4; > +SET @@sql_mode=@save_sql_mode; > +# End of 10.2 tests > diff --git a/sql/handler.cc b/sql/handler.cc > index ae63640ae5c..d307433cf5d 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -2648,8 +2648,7 @@ int handler::ha_rnd_pos(uchar *buf, uchar *pos) > DBUG_ENTER("handler::ha_rnd_pos"); > DBUG_ASSERT(table_share->tmp_table != NO_TMP_TABLE || > m_lock_type != F_UNLCK); > - /* TODO: Find out how to solve ha_rnd_pos when finding duplicate update. */ > - /* DBUG_ASSERT(inited == RND); */ > + DBUG_ASSERT(inited == RND); > > TABLE_IO_WAIT(tracker, m_psi, PSI_TABLE_FETCH_ROW, MAX_KEY, 0, > { result= rnd_pos(buf, pos); }) > @@ -3307,6 +3306,10 @@ void handler::get_auto_increment(ulonglong offset, ulonglong increment, > ulonglong nr; > int error; > MY_BITMAP *old_read_set; > + bool rnd_inited= (inited == RND); > + when is that true? > diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc > index 8a9dd083911..cc1649b3c92 100644 > --- a/sql/item_subselect.cc > +++ b/sql/item_subselect.cc > @@ -5855,12 +5855,17 @@ Ordered_key::cmp_keys_by_row_data_and_rownum(Ordered_key *key, > } > > > -void Ordered_key::sort_keys() > +bool Ordered_key::sort_keys() > { > + /* Prepare table for random positioning (importent for innodb) */ 1) typo: "important" but 2) I don't think you need a comment here at all now. There's an assert in ha_rnd_pos(), it's rather more clear and more reliable anyway. > + if (tbl->file->ha_rnd_init(0)) > + return TRUE; > my_qsort2(key_buff, (size_t) key_buff_elements, sizeof(rownum_t), > (qsort2_cmp) &cmp_keys_by_row_data_and_rownum, (void*) this); > /* Invalidate the current row position. */ > cur_key_idx= HA_POS_ERROR; > + tbl->file->ha_rnd_end(); > + return FALSE; > } > > > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index 307473cecbd..5593580dbca 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -3177,6 +3193,10 @@ bool Delayed_insert::handle_inserts(void) > max_rows= ULONG_MAX; // Do as much as possible > } > > + /* Prepare table for random positioning (importent for innodb) */ same > + if (table->file->ha_rnd_init(0)) > + goto err; what's that for? duplicate key error message? > + > /* > We can't use row caching when using the binary log because if > we get a crash, then binary log will contain rows that are not yet > @@ -3649,7 +3671,12 @@ select_insert::prepare(List<Item> &values, SELECT_LEX_UNIT *u) > > thd->cuted_fields=0; > if (info.ignore || info.handle_duplicates != DUP_ERROR) > + { > table->file->extra(HA_EXTRA_IGNORE_DUP_KEY); > + if (table->file->ha_table_flags() & HA_DUPLICATE_POS && > + table->file->ha_rnd_init(0)) > + DBUG_RETURN(1); here and in other places, you don't do my_error(). Is there some catch-all my_error somewhere up the stack or it'll be the usual assert in Diagnostic_area? > + } > if (info.handle_duplicates == DUP_REPLACE && > (!table->triggers || !table->triggers->has_delete_triggers())) > table->file->extra(HA_EXTRA_WRITE_CAN_REPLACE); > @@ -3929,6 +3959,11 @@ void select_insert::abort_result_set() { > if (thd->locked_tables_mode <= LTM_LOCK_TABLES) > table->file->ha_end_bulk_insert(); > > + if (table->file->inited) why not "if (table->file->ha_table_flags() & HA_DUPLICATE_POS)" ? > + table->file->ha_rnd_end(); > + table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); > + table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); > + > /* > If at least one row has been inserted/modified and will stay in > the table (the table doesn't have transactions) we must write to > diff --git a/sql/sql_load.cc b/sql/sql_load.cc > index 8c2f17dac3f..3283b962f65 100644 > --- a/sql/sql_load.cc > +++ b/sql/sql_load.cc > @@ -638,6 +638,13 @@ int mysql_load(THD *thd,sql_exchange *ex,TABLE_LIST *table_list, > > thd->abort_on_warning= !ignore && thd->is_strict_mode(); > > + /* Prepare table for random positioning (importent for innodb) */ same > + if ((table_list->table->file->ha_table_flags() & HA_DUPLICATE_POS) && > + table_list->table->file->ha_rnd_init(0)) > + { > + error= 1; > + goto err; > + } > thd_progress_init(thd, 2); > if (table_list->table->validate_default_values_of_unset_fields(thd)) > { > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 91a6445c870..4c401ea9011 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -20317,6 +20317,12 @@ end_unique_update(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), > table->file->print_error(error,MYF(0)); /* purecov: inspected */ > DBUG_RETURN(NESTED_LOOP_ERROR); /* purecov: inspected */ > } > + /* Prepare table for random positioning */ > + bool rnd_inited= (table->file->inited == handler::RND); when is that true? and when false? > + if (!rnd_inited && > + ((error= table->file->ha_index_end()) || > + (error= table->file->ha_rnd_init(0)))) > + DBUG_RETURN(NESTED_LOOP_ERROR); > if (table->file->ha_rnd_pos(table->record[1],table->file->dup_ref)) > { > table->file->print_error(error,MYF(0)); /* purecov: inspected */ > diff --git a/sql/sql_update.cc b/sql/sql_update.cc > index 11ffa684216..6d4c11d494a 100644 > --- a/sql/sql_update.cc > +++ b/sql/sql_update.cc > @@ -211,6 +211,10 @@ static void prepare_record_for_error_message(int error, TABLE *table) > bitmap_union(table->read_set, &unique_map); > /* Tell the engine about the new set. */ > table->file->column_bitmaps_signal(); > + /* Prepare table for random positioning (importent for innodb) */ same > + if (table->file->ha_index_or_rnd_end() || > + table->file->ha_rnd_init(0)) > + DBUG_VOID_RETURN; > /* Read record that is identified by table->file->ref. */ > (void) table->file->ha_rnd_pos(table->record[1], table->file->ref); > /* Copy the newly read columns into the new record. */ > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 34098adf32b: Get rid of rea_create_table()
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 34098adf32b (mariadb-10.4.1-138-g34098adf32b) > parent(s): d953dc02d9a > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-07 20:04:51 +0400 > message: > > Get rid of rea_create_table() > > Moved rea_create_table() to the sole caller. > > Visible difference: regular ALTER TABLE doesn't create .frm for temporary > and discoverable tables anymore. > > Also ha_create_partitioning_metadata(CHF_CREATE_FLAG) does cleanup on > error now. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] d953dc02d9a: Fix inplace ALTER TABLE to not register tmp table
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: d953dc02d9a (mariadb-10.4.1-137-gd953dc02d9a) > parent(s): 9496394002f > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-07 20:04:38 +0400 > message: > > Fix inplace ALTER TABLE to not register tmp table > > Do not register intermediate tables created by inplace ALTER TABLE in > THD::temporary_tables. > > For inplace ALTER TABLE moved .frm creation to > create_table_for_inplace_alter(). For regular ALTER TABLE it is (and was > redundantly) created by ha_create_table(). > > Removed open_in_engine argument of create_and_open_tmp_table() and > open_temporary_table(): it became unused after this patch. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. Looks ok code wise. But I don't see what are you gaining by this transformation. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Self Introduction (gsoc)
by shreya pohekar 14 Mar '19

14 Mar '19

Hey! I am Shreya Pohekar. I am currently in my 3rd year of BE in Computer Science and Engineering from University Institute of Technology, RGPV Bhopal. I have a conceptual proficiency on Java and C++. On scrolling through the projects of mariadb, i found most of the projects interesting. But i would like to contribute to the project, "Implement multi-table UPDATE/DELETE returning a result set".@Igor Babaev, If you can guide me on where to get started, that would be great. Looking forward to hearing from you. Thank you.

1 0

[Maria-developers] Self Introduction (gsoc)
by shreya pohekar 14 Mar '19

14 Mar '19

Hey! I am Shreya Pohekar. I am currently in my 3rd year of BE in Computer Science and Engineering from University Institute of Technology, RGPV Bhopal. I have a conceptual proficiency on Java and C++. On scrolling through the projects of mariadb, i found most of the projects interesting. But i would like to contribute to the project, "Implement multi-table UPDATE/DELETE returning a result set". If you can guide me on where to get started, that would be great. Looking forward to hearing from you. Thank you.

1 0

Re: [Maria-developers] 9496394002f: Removed redundant partitioning check
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 9496394002f (mariadb-10.4.1-136-g9496394002f) > parent(s): 311beaffa63 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-06 13:56:42 +0400 > message: > > Removed redundant partitioning check > > This check was introduced in 602a222 and then became redundant in ad1553e, > where we attempt to open a table even for non-copy algorithms. > > Added missing test case from 602a222. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 311beaffa63: Simplified dd_recreate_table()
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 311beaffa63 (mariadb-10.4.1-135-g311beaffa63) > parent(s): 3323b013c13 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-06 13:56:42 +0400 > message: > > Simplified dd_recreate_table() > > It is used only with persistent tables, so remove path argument, which > was introduced in 9594107f and became useless after ce6a63e. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. ok to push Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 3323b013c13: Removed redundant SE lock for tmp tables
by Sergei Golubchik 14 Mar '19

14 Mar '19

Hi, Sergey! On Mar 13, Sergey Vojtovich wrote: > revision-id: 3323b013c13 (mariadb-10.4.1-134-g3323b013c13) > parent(s): 4b3656a44d6 > author: Sergey Vojtovich <svoj(a)mariadb.org> > committer: Sergey Vojtovich <svoj(a)mariadb.org> > timestamp: 2019-02-06 13:56:28 +0400 > message: > > Removed redundant SE lock for tmp tables > > CREATE TEMPORARY TABLE locks SE plugin 6 times. 5 of these locks are > released by the end of the statement. And only 1 acquired by > init_from_binary_frm_image() / plugin_lock() remains. > > The lock removed in this patch was clearly redundant. > > Part of MDEV-17805 - Remove InnoDB cache for temporary tables. > > diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc > index 917a85e6c3b..438b670b5f5 100644 > --- a/sql/temporary_tables.cc > +++ b/sql/temporary_tables.cc > @@ -951,8 +947,6 @@ TMP_TABLE_SHARE *THD::create_temporary_table(handlerton *hton, > init_tmp_table_share(this, share, saved_key_cache, key_length, > strend(saved_key_cache) + 1, tmp_path); > > - share->db_plugin= ha_lock_engine(this, hton); > - I suppose, share->db_plugin will be set in the following share->init_from_binary_frm_image ? Ok to push. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 14c2f90ad08: Idempotent INSERT events for system versioning
by Aleksey Midenkov 13 Mar '19

13 Mar '19

Hi, Sergei, Thank you for observations! This task is in progress. While doing it I found out that RBR doesn't replicate timestamp fractions. That is: it gets seconds from event timestamp marker and adds fractions from system time. This causes bugs in System Versioning. More on it in Bug 6 here: https://github.com/tempesta-tech/mariadb/issues/578#issuecomment-470533050 I propose to fix it by adding timestamp fractions field to Log_event header after FLAGS_OFFSET. To support backwards compatibility it is needed to increment fdle->binlog_version and probably add new flag LOG_EVENT_HAS_SEC_PART_F (for support sending Log_event without fractions). Do you agree? On Thu, Jan 31, 2019 at 1:10 PM Sergei Golubchik <serg(a)mariadb.com> wrote: > Hi, Aleksey! > > On Jan 31, Aleksey Midenkov wrote: > > revision-id: 14c2f90ad08 (versioning-1.0.7-1-g14c2f90ad08) > > parent(s): a8efe7ab1f2 > > author: Aleksey Midenkov <midenok(a)gmail.com> > > committer: Aleksey Midenkov <midenok(a)gmail.com> > > timestamp: 2018-11-17 16:30:10 +0300 > > message: > > > > Idempotent INSERT events for system versioning > > > > Case 1: Rows_log_event::write_row() always overwrite historical row. > > > > Related to MDEV-16370. > > I realized that the whole RBR logging of system versioned tables is > wrong. > > For an update you log update_row event and write_row event separately > and execute them separately. It allows trivially to manipulate the history. > See a test case attached. > > To fix this, the slave should ignore all events that modify the history. > And generate them locally. Say, an update on the master generates > update_row and write_row events. The slave executes update_row event > which updates a current row and this also should generate the historical > row. Then the slave ignores write_row event. > > Of course, as an optimization, the master should not generate historical > write_row events, but as my test case shows, they can be forged, so the > slave should ignore them anyway, if they happen to come. > > I've created a new https://jira.mariadb.org/browse/MDEV-18432 for that, > let's not have it tied to MDEV-16370. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org > -- All the best, Aleksey Midenkov @midenok

3 7

Re: [Maria-developers] 1aa6d5bdbec: MDEV-18888 Server crashes in Item_field::register_field_in_read_map upon...
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! On Mar 12, Sachin Setiya wrote: > revision-id: 1aa6d5bdbec (mariadb-10.4.3-63-g1aa6d5bdbec) > parent(s): b5d2e8577ef > author: Sachin Setiya <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:58 +0530 > message: > > MDEV-18888 Server crashes in Item_field::register_field_in_read_map upon... > MODIFY COLUMN > > Do Not create prefix field for long unique key > > diff --git a/sql/table.cc b/sql/table.cc > index 4da873b2e8f..5ca284fd8dd 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -3709,7 +3709,33 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, > Field *field= key_part->field= outparam->field[key_part->fieldnr - 1]; > > if (field->key_length() != key_part->length && > - !(field->flags & BLOB_FLAG)) > + !(field->flags & BLOB_FLAG) && > + /* > + MDEV-18888 > + We will not make copy of the field because if we do this this this will > + make this test case to fail > + CREATE TABLE t1 ( > + a CHAR(128), > + b CHAR(128) AS (a), > + c varchar(5000), > + UNIQUE(c,b(64)) > + ) ENGINE=InnoDB; > + And the reason for this is because Item_func_hash will Item_field based on > + this new field. But if this field is virtual field then since it is created > + before calling parse_vcol_defs so it does not have field->vcol_info->expr item > + initiated. And this will make segfault when field->vcol_info->expr is accessed. > + So the question may be why similar unique with just virtual column is not crashing ? > + CREATE TABLE t2 ( > + a CHAR(128), > + b CHAR(128) AS (a), > + c char(128) as (a+b), > + UNIQUE(a, c(64)) > + ) ENGINE=InnoDB; > + In this case also new prefix field will be created whose vcol_info->expr will be NULL. > + But Since this key is not a long unique in innodb_base_col_setup field will be > + table->field[i] which will have not null vcol_info->expr > + */ > + key_info->algorithm != HA_KEY_ALG_LONG_HASH) don't put a comment (particularly a long one) inside the if(), move it to be before the if(), please. and I'd used a simpler comment, like "there's no need to create a prefix Field for HA_KEY_ALG_LONG_HASH indexes, as they implement prefixing via Iten_func_left anyway (see parse_vcol_defs()" > { > /* > We are using only a prefix of the column as a key: Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] b5d2e8577ef: MDEV-18889 Long unique on virtual fields crashes server
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! On Mar 12, Sachin Setiya wrote: > revision-id: b5d2e8577ef (mariadb-10.4.3-62-gb5d2e8577ef) > parent(s): 202d1b0c897 > author: Sachin Setiya <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:58 +0530 > message: > > MDEV-18889 Long unique on virtual fields crashes server > > When we receive record from ha_index_read_map it does not have vfield > values. So we need to update it. > > diff --git a/sql/handler.cc b/sql/handler.cc > index 0746f8a81fc..c4f81c6432e 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -6522,6 +6522,26 @@ static int wsrep_after_row(THD *thd) > } > #endif /* WITH_WSREP */ > > +/* > + Long Unique can have virtual fields. But when we receive record from > + ha_index_read_map it does not have vfield values. So we need to update it. > + IN table Table Object > + IN handler Handler Object > + OUT updated Set to true on successful update Doxygen style for new comments, please. And feel free to rewrite old comments to use Doxygen style whenever you need to edit them anyway. > +*/ > +static void update_table_vcol(TABLE *table, handler *h, bool& updated) > +{ > + uint length= table->s->reclength; > + uchar temp_record[length]; Not a good idea: 1. you'll get compiler warnings for using VLA (-Wvla: Warn if a variable-length array is used") 2. length can be rather large, stack can be rather small. 3. two big memcpy's per row read. Two other options: 1. use field->move(check_unique_buf - record[0]) to move field pointers to check_unique_buf. You can do it once, at the beginning and restore them at the end, no need to do it per read. See TABLE::move_fields() 2. Copy record[0] to check_unique_buf(). And read potential duplicates into record[0]. Then you won't need to do anything special at all, ha_index_read_map() will update vcols automatically. Just don't forget to restore record[0] at the end. See store_record(), restore_record(). I think the second approach might be easier. > + memcpy(temp_record, table->record[0], length); > + restore_record(table, check_unique_buf); > + if (!table->update_virtual_fields(h, VCOL_UPDATE_FOR_READ)) > + updated= true; > + store_record(table, check_unique_buf); > + memcpy(table->record[0], temp_record, length); > + > +} > + Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 003abb6d9dc: MDEV-18799 Long unique does not work after failed alter table
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! On Mar 12, Sachin Setiya wrote: > revision-id: 003abb6d9dc (mariadb-10.4.3-58-g003abb6d9dc) > parent(s): e2f8f968785 > author: sachin <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:57 +0530 > message: > > MDEV-18799 Long unique does not work after failed alter table > > Restore table->key_info after calling setup_keyinfo_hash in > mysql_prepare_alter_table. > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index e721a868124..2d86035b7fd 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -7923,6 +7923,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, > Field **f_ptr,*field; > MY_BITMAP *dropped_fields= NULL; // if it's NULL - no dropped fields > bool drop_period= false; > + bool long_hash_key= false; > DBUG_ENTER("mysql_prepare_alter_table"); > > /* > @@ -8299,10 +8300,14 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, > */ > for (uint i=0 ; i < table->s->keys ; i++,key_info++) > { > + long_hash_key= false; better declare long_hash_key here, you don't use it on a larger scope. > if (key_info->flags & HA_INVISIBLE_KEY) > continue; > if (key_info->algorithm == HA_KEY_ALG_LONG_HASH) > + { > setup_keyinfo_hash(key_info); > + long_hash_key= true; > + } > const char *key_name= key_info->name.str; > Alter_drop *drop; > drop_it.rewind(); > @@ -8476,8 +8481,12 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, > &key_parts, key_info->option_list, DDL_options()); > new_key_list.push_back(key, thd->mem_root); > } > - if (key_info->algorithm == HA_KEY_ALG_LONG_HASH) > + if (long_hash_key) > + { > + key_info->algorithm= HA_KEY_ALG_LONG_HASH; > + key_info->flags&= ~HA_NOSAME; perhaps you could move HA_NOSAME manipulations into [re_]setup_keyinfo_hash ? > re_setup_keyinfo_hash(key_info); > + } > } > { > Key *key; > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 202d1b0c897: MDEV-18809 Server crash in fields_in_hash_keyinfo or Assertion...
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! On Mar 12, Sachin Setiya wrote: > revision-id: 202d1b0c897 (mariadb-10.4.3-61-g202d1b0c897) > parent(s): a4b8e0c30d1 > author: sachin <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:58 +0530 > message: > > MDEV-18809 Server crash in fields_in_hash_keyinfo or Assertion... > `key_info->key_part->field->flags & (1<< 30)' failed in setup_keyinfo_hash put the subject all on one line please, even when it's (unfortunately) long > > Move calling setup_keyinfo_hash until all continue is exhausted. okay. but there's one `goto err` still in the loop. I suspect it might be a problem, try to do a test case with ALTER TABLE DROP COLUMN from a multi-column long unique constraint. > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index a9c4c71331a..58f175d7a29 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -8304,11 +8304,6 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, > long_hash_key= false; > if (key_info->flags & HA_INVISIBLE_KEY) > continue; > - if (key_info->algorithm == HA_KEY_ALG_LONG_HASH) > - { > - setup_keyinfo_hash(key_info); > - long_hash_key= true; > - } > const char *key_name= key_info->name.str; > Alter_drop *drop; > drop_it.rewind(); > @@ -8339,6 +8334,11 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, > continue; > } > > + if (key_info->algorithm == HA_KEY_ALG_LONG_HASH) > + { > + setup_keyinfo_hash(key_info); > + long_hash_key= true; > + } > const char *dropped_key_part= NULL; > KEY_PART_INFO *key_part= key_info->key_part; > key_parts.empty(); > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] a4b8e0c30d1: MDEV-18800 Server crash in instant_alter_column_possible or Assertion...
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! ok to push On Mar 12, Sachin Setiya wrote: > revision-id: a4b8e0c30d1 (mariadb-10.4.3-60-ga4b8e0c30d1) > parent(s): d8e4463d5dc > author: sachin <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:57 +0530 > message: > > MDEV-18800 Server crash in instant_alter_column_possible or Assertion... > `!pk->has_virtual()' failed in instant_alter_column_possible upon adding key > > Hash key can't be primary key. > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 2d86035b7fd..a9c4c71331a 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -4179,6 +4179,7 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, > } > if (is_hash_field_needed || > (key_info->algorithm == HA_KEY_ALG_HASH && > + key->type != Key::PRIMARY && > key_info->flags & HA_NOSAME && > !(file->ha_table_flags() & HA_CAN_HASH_KEYS ) && > file->ha_table_flags() & HA_CAN_VIRTUAL_COLUMNS)) > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] e2f8f968785: MDEV-18790 Server crash in fields_in_hash_keyinfo after unsuccessful...
by Sergei Golubchik 12 Mar '19

12 Mar '19

Hi, Sachin! Ok to push, just one minor detail, see below On Mar 12, Sachin Setiya wrote: > revision-id: e2f8f968785 (mariadb-10.4.3-57-ge2f8f968785) > parent(s): 6d68a3464e9 > author: sachin <sachin.setiya(a)mariadb.com> > committer: sachin <sachin.setiya(a)mariadb.com> > timestamp: 2019-03-12 19:29:57 +0530 > message: > > MDEV-18790 Server crash in fields_in_hash_keyinfo after unsuccessful... > attempt to drop BLOB with long index > > Restore long table->key_info, So that in the case of unsuccessful table > alter table->key_info should have a correct value. In the case of successful > table alter old table is flushed so that is why we don't see this error in > the case of successful alter. > > diff --git a/mysql-test/main/long_unique_bugs.result b/mysql-test/main/long_unique_bugs.result > index 5de7566c8a3..1544565d95e 100644 > --- a/mysql-test/main/long_unique_bugs.result > +++ b/mysql-test/main/long_unique_bugs.result > @@ -57,3 +57,9 @@ create table t (b blob, unique(b)) engine=myisam; > insert into t values ('foo'); > replace into t values ('foo'); > drop table t; > +CREATE TABLE t1 (f INT, x BLOB, UNIQUE (x)); > +INSERT INTO t1 VALUES (1,'foo'); > +ALTER TABLE t1 DROP x, ALGORITHM=INPLACE; > +Got one of the listed errors > +UPDATE t1 SET x = 'bar'; > +DROP TABLE t1; > diff --git a/mysql-test/main/long_unique_bugs.test b/mysql-test/main/long_unique_bugs.test > index 0246ea7962a..14c002f998c 100644 > --- a/mysql-test/main/long_unique_bugs.test > +++ b/mysql-test/main/long_unique_bugs.test > @@ -72,3 +72,13 @@ create table t (b blob, unique(b)) engine=myisam; > insert into t values ('foo'); > replace into t values ('foo'); > drop table t; > + > +# > +# MDEV-18790 Server crash in fields_in_hash_keyinfo after unsuccessful attempt to drop BLOB with long index > +# > +CREATE TABLE t1 (f INT, x BLOB, UNIQUE (x)); > +INSERT INTO t1 VALUES (1,'foo'); > +--error ER_ALTER_OPERATION_NOT_SUPPORTED,ER_ALTER_OPERATION_NOT_SUPPORTED_REASON this makes the test to accept any of the listed error messages and the result file will have "Got one of the listed errors". It's useful when a test can indeed see different errors in a certain place (Elena has probably run the test with different storage engines, and has seen different errors for MyISAM and InnoDB). But this final committed test case is very deterministic, and as far as I can see there can be only one possible error code here. It's generally better to have just this one error code in the `--error` directive and the correct complete error message text in the result file. > +ALTER TABLE t1 DROP x, ALGORITHM=INPLACE; > +UPDATE t1 SET x = 'bar'; > +DROP TABLE t1; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 190a29d06: Make Lock Tree's lock waits use externally provided mutexes/conditions
by jocelyn fournier 12 Mar '19

12 Mar '19

Hi Sergei! Just wondering why keeping the toku_ prefix after renaming toku_* to toku_external_* since it’s also used by other engine like RockDB? BR, Jocelyn Fournier > Le 12 mars 2019 à 09:52, Sergei Petrunia <psergey(a)askmonty.org> a écrit : > > revision-id: 190a29d06f4e79d2df4cb513944ac34bd133caa0 (v5.8-1024-g190a29d06) > parent(s): 7a8c777d4880cde8fd96135f6cc216223371a9f4 > author: Sergei Petrunia > committer: Sergei Petrunia > timestamp: 2019-03-12 11:52:17 +0300 > message: > > Make Lock Tree's lock waits use externally provided mutexes/conditions > > In MyRocks, this allows: > - SHOW PROCESSLIST to show "waiting for row lock" for waiting threads > - KILL statement to abort the lock wait in the same way as it does it with > point locking (without use of handlerton::kill_connection() method) > > - Remove previously added TransactionDB::KillLockWait() method as it is > no longer needed. > > --- > include/rocksdb/utilities/transaction_db.h | 1 - > .../transactions/pessimistic_transaction_db.cc | 12 +++- > .../transactions/pessimistic_transaction_db.h | 4 -- > .../range_locking/locktree/lock_request.cc | 35 ++++++---- > .../range_locking/locktree/lock_request.h | 9 ++- > .../range_locking/locktree/locktree.cc | 12 ++-- > .../transactions/range_locking/locktree/locktree.h | 13 ++-- > .../transactions/range_locking/locktree/manager.cc | 17 +++-- > .../portability/toku_external_pthread.h | 81 ++++++++++++++++++++++ > utilities/transactions/transaction_lock_mgr.cc | 15 ++-- > utilities/transactions/transaction_lock_mgr.h | 6 +- > 11 files changed, 155 insertions(+), 50 deletions(-) > > diff --git a/include/rocksdb/utilities/transaction_db.h b/include/rocksdb/utilities/transaction_db.h > index 2a98a8d3d..fb3c6a88c 100644 > --- a/include/rocksdb/utilities/transaction_db.h > +++ b/include/rocksdb/utilities/transaction_db.h > @@ -291,7 +291,6 @@ class TransactionDB : public StackableDB { > bool use_range_locking; > virtual RangeLockMgrControl* get_range_lock_manager() { return nullptr; } > > - virtual void KillLockWait(TransactionID txnid){}; > protected: > // To Create an TransactionDB, call Open() > // The ownership of db is transferred to the base StackableDB > diff --git a/utilities/transactions/pessimistic_transaction_db.cc b/utilities/transactions/pessimistic_transaction_db.cc > index d44f329e5..308187e13 100644 > --- a/utilities/transactions/pessimistic_transaction_db.cc > +++ b/utilities/transactions/pessimistic_transaction_db.cc > @@ -41,7 +41,11 @@ PessimisticTransactionDB::PessimisticTransactionDB( > ? txn_db_options_.custom_mutex_factory > : std::shared_ptr<TransactionDBMutexFactory>( > new TransactionDBMutexFactoryImpl())), > - range_lock_mgr_(this) { > + range_lock_mgr_(this, > + txn_db_options_.custom_mutex_factory? > + txn_db_options_.custom_mutex_factory : > + std::shared_ptr<TransactionDBMutexFactory>( > + new TransactionDBMutexFactoryImpl())) { > assert(db_impl_ != nullptr); > info_log_ = db_impl_->GetDBOptions().info_log; > } > @@ -73,7 +77,11 @@ PessimisticTransactionDB::PessimisticTransactionDB( > ? txn_db_options_.custom_mutex_factory > : std::shared_ptr<TransactionDBMutexFactory>( > new TransactionDBMutexFactoryImpl())), > - range_lock_mgr_(this) { > + range_lock_mgr_(this, > + txn_db_options_.custom_mutex_factory > + ? txn_db_options_.custom_mutex_factory > + : std::shared_ptr<TransactionDBMutexFactory>( > + new TransactionDBMutexFactoryImpl())) { > assert(db_impl_ != nullptr); > } > > diff --git a/utilities/transactions/pessimistic_transaction_db.h b/utilities/transactions/pessimistic_transaction_db.h > index 07ed3fed2..388ed4099 100644 > --- a/utilities/transactions/pessimistic_transaction_db.h > +++ b/utilities/transactions/pessimistic_transaction_db.h > @@ -127,10 +127,6 @@ class PessimisticTransactionDB : public TransactionDB { > virtual void UpdateCFComparatorMap(const std::vector<ColumnFamilyHandle*>&) {} > virtual void UpdateCFComparatorMap(ColumnFamilyHandle*) {} > > - void KillLockWait(TransactionID txnid) override { > - if (use_range_locking) > - range_lock_mgr_.KillLockWait(txnid); > - } > protected: > DBImpl* db_impl_; > std::shared_ptr<Logger> info_log_; > diff --git a/utilities/transactions/range_locking/locktree/lock_request.cc b/utilities/transactions/range_locking/locktree/lock_request.cc > index 0b96b2051..84cfdbf62 100644 > --- a/utilities/transactions/range_locking/locktree/lock_request.cc > +++ b/utilities/transactions/range_locking/locktree/lock_request.cc > @@ -59,7 +59,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved. > namespace toku { > > // initialize a lock request's internals > -void lock_request::create(void) { > +void lock_request::create(toku_external_mutex_factory_t mutex_factory) { > m_txnid = TXNID_NONE; > m_conflicting_txnid = TXNID_NONE; > m_start_time = 0; > @@ -75,7 +75,9 @@ void lock_request::create(void) { > m_state = state::UNINITIALIZED; > m_info = nullptr; > > - toku_cond_init(*lock_request_m_wait_cond_key, &m_wait_cond, nullptr); > + //psergey-todo: this condition is for interruptible wait > + // note: moved to here from lock_request::create: > + toku_external_cond_init(mutex_factory, &m_wait_cond); > > m_start_test_callback = nullptr; > m_start_before_pending_test_callback = nullptr; > @@ -89,13 +91,14 @@ void lock_request::destroy(void) { > m_state = state::DESTROYED; > toku_destroy_dbt(&m_left_key_copy); > toku_destroy_dbt(&m_right_key_copy); > - toku_cond_destroy(&m_wait_cond); > + toku_external_cond_destroy(&m_wait_cond); > } > > // set the lock request parameters. this API allows a lock request to be reused. > void lock_request::set(locktree *lt, TXNID txnid, const DBT *left_key, const DBT *right_key, lock_request::type lock_type, bool big_txn, void *extra) { > invariant(m_state != state::PENDING); > m_lt = lt; > + > m_txnid = txnid; > m_left_key = left_key; > m_right_key = right_key; > @@ -190,13 +193,13 @@ int lock_request::start(void) { > m_conflicting_txnid = conflicts.get(0); > if (m_start_before_pending_test_callback) > m_start_before_pending_test_callback(); > - toku_mutex_lock(&m_info->mutex); > + toku_external_mutex_lock(&m_info->mutex); > insert_into_lock_requests(); > if (deadlock_exists(conflicts)) { > remove_from_lock_requests(); > r = DB_LOCK_DEADLOCK; > } > - toku_mutex_unlock(&m_info->mutex); > + toku_external_mutex_unlock(&m_info->mutex); > if (m_start_test_callback) > m_start_test_callback(); // test callback > } > @@ -221,7 +224,7 @@ int lock_request::wait(uint64_t wait_time_ms, uint64_t killed_time_ms, int (*kil > uint64_t t_start = t_now; > uint64_t t_end = t_start + wait_time_ms * 1000; > > - toku_mutex_lock(&m_info->mutex); > + toku_external_mutex_lock(&m_info->mutex); > > // check again, this time locking out other retry calls > if (m_state == state::PENDING) { > @@ -252,10 +255,14 @@ int lock_request::wait(uint64_t wait_time_ms, uint64_t killed_time_ms, int (*kil > if (t_wait > t_end) > t_wait = t_end; > } > + /* > + PORT: don't need to compute "time when the wait should end" anymore. > struct timespec ts = {0, 0}; > ts.tv_sec = t_wait / 1000000; > ts.tv_nsec = (t_wait % 1000000) * 1000; > - int r = toku_cond_timedwait(&m_wait_cond, &m_info->mutex, &ts); > + */ > + int r = toku_external_cond_timedwait(&m_wait_cond, &m_info->mutex, > + int32_t(wait_time_ms)*1000); > invariant(r == 0 || r == ETIMEDOUT); > > t_now = toku_current_time_microsec(); > @@ -279,7 +286,7 @@ int lock_request::wait(uint64_t wait_time_ms, uint64_t killed_time_ms, int (*kil > m_info->counters.long_wait_count += 1; > m_info->counters.long_wait_time += duration; > } > - toku_mutex_unlock(&m_info->mutex); > + toku_external_mutex_unlock(&m_info->mutex); > > invariant(m_state == state::COMPLETE); > return m_complete_r; > @@ -332,7 +339,7 @@ int lock_request::retry(GrowableArray<TXNID> *conflicts_collector) { > complete(r); > if (m_retry_test_callback) > m_retry_test_callback(); // test callback > - toku_cond_broadcast(&m_wait_cond); > + toku_external_cond_broadcast(&m_wait_cond); > } else { > m_conflicting_txnid = conflicts.get(0); > add_conflicts_to_waits(&conflicts, conflicts_collector); > @@ -393,7 +400,7 @@ void lock_request::retry_all_lock_requests( > } > > void lock_request::retry_all_lock_requests_info(lt_lock_request_info *info, GrowableArray<TXNID> *collector) { > - toku_mutex_lock(&info->mutex); > + toku_external_mutex_lock(&info->mutex); > // retry all of the pending lock requests. > for (size_t i = 0; i < info->pending_lock_requests.size();) { > lock_request *request; > @@ -412,7 +419,7 @@ void lock_request::retry_all_lock_requests_info(lt_lock_request_info *info, Grow > > // future threads should only retry lock requests if some still exist > info->should_retry_lock_requests = info->pending_lock_requests.size() > 0; > - toku_mutex_unlock(&info->mutex); > + toku_external_mutex_unlock(&info->mutex); > } > > void lock_request::add_conflicts_to_waits(txnid_set *conflicts, > @@ -444,12 +451,12 @@ void *lock_request::get_extra(void) const { > void lock_request::kill_waiter(void) { > remove_from_lock_requests(); > complete(DB_LOCK_NOTGRANTED); > - toku_cond_broadcast(&m_wait_cond); > + toku_external_cond_broadcast(&m_wait_cond); > } > > void lock_request::kill_waiter(locktree *lt, void *extra) { > lt_lock_request_info *info = lt->get_lock_request_info(); > - toku_mutex_lock(&info->mutex); > + toku_external_mutex_lock(&info->mutex); > for (size_t i = 0; i < info->pending_lock_requests.size(); i++) { > lock_request *request; > int r = info->pending_lock_requests.fetch(i, &request); > @@ -458,7 +465,7 @@ void lock_request::kill_waiter(locktree *lt, void *extra) { > break; > } > } > - toku_mutex_unlock(&info->mutex); > + toku_external_mutex_unlock(&info->mutex); > } > > // find another lock request by txnid. must hold the mutex. > diff --git a/utilities/transactions/range_locking/locktree/lock_request.h b/utilities/transactions/range_locking/locktree/lock_request.h > index 8b9529344..b17b639c8 100644 > --- a/utilities/transactions/range_locking/locktree/lock_request.h > +++ b/utilities/transactions/range_locking/locktree/lock_request.h > @@ -84,7 +84,7 @@ public: > }; > > // effect: Initializes a lock request. > - void create(void); > + void create(toku_external_mutex_factory_t mutex_factory); > > // effect: Destroys a lock request. > void destroy(void); > @@ -171,7 +171,7 @@ public: > int m_complete_r; > state m_state; > > - toku_cond_t m_wait_cond; > + toku_external_cond_t m_wait_cond; > > bool m_big_txn; > > @@ -227,6 +227,9 @@ public: > > friend class lock_request_unit_test; > }; > -ENSURE_POD(lock_request); > +// PORT: lock_request is not a POD anymore due to use of toku_external_cond_t > +// This is ok as the PODness is not really required: lock_request objects are > +// not moved in memory or anything. > +//ENSURE_POD(lock_request); > > } /* namespace toku */ > diff --git a/utilities/transactions/range_locking/locktree/locktree.cc b/utilities/transactions/range_locking/locktree/locktree.cc > index 4ecfb2d26..9b530c7b0 100644 > --- a/utilities/transactions/range_locking/locktree/locktree.cc > +++ b/utilities/transactions/range_locking/locktree/locktree.cc > @@ -74,7 +74,9 @@ namespace toku { > // but does nothing based on the value of the reference count - it is > // up to the user of the locktree to destroy it when it sees fit. > > -void locktree::create(locktree_manager *mgr, DICTIONARY_ID dict_id, const comparator &cmp) { > +void locktree::create(locktree_manager *mgr, DICTIONARY_ID dict_id, > + const comparator &cmp, > + toku_external_mutex_factory_t mutex_factory) { > m_mgr = mgr; > m_dict_id = dict_id; > > @@ -91,14 +93,14 @@ void locktree::create(locktree_manager *mgr, DICTIONARY_ID dict_id, const compar > m_sto_end_early_count = 0; > m_sto_end_early_time = 0; > > - m_lock_request_info.init(); > + m_lock_request_info.init(mutex_factory); > } > > -void lt_lock_request_info::init(void) { > +void lt_lock_request_info::init(toku_external_mutex_factory_t mutex_factory) { > pending_lock_requests.create(); > pending_is_empty = true; > ZERO_STRUCT(mutex); > - toku_mutex_init(*locktree_request_info_mutex_key, &mutex, nullptr); > + toku_external_mutex_init(mutex_factory, &mutex); > retry_want = retry_done = 0; > ZERO_STRUCT(counters); > ZERO_STRUCT(retry_mutex); > @@ -124,7 +126,7 @@ void locktree::destroy(void) { > > void lt_lock_request_info::destroy(void) { > pending_lock_requests.destroy(); > - toku_mutex_destroy(&mutex); > + toku_external_mutex_destroy(&mutex); > toku_mutex_destroy(&retry_mutex); > toku_cond_destroy(&retry_cv); > } > diff --git a/utilities/transactions/range_locking/locktree/locktree.h b/utilities/transactions/range_locking/locktree/locktree.h > index 4f0530855..5ff4f7449 100644 > --- a/utilities/transactions/range_locking/locktree/locktree.h > +++ b/utilities/transactions/range_locking/locktree/locktree.h > @@ -55,6 +55,7 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved. > > #include <db.h> > #include <toku_pthread.h> > +#include <toku_external_pthread.h> > #include <toku_time.h> > > #include <ft/comparator.h> > @@ -102,7 +103,7 @@ namespace toku { > struct lt_lock_request_info { > omt<lock_request *> pending_lock_requests; > std::atomic_bool pending_is_empty; > - toku_mutex_t mutex; > + toku_external_mutex_t mutex; > bool should_retry_lock_requests; > lt_counters counters; > std::atomic_ullong retry_want; > @@ -111,7 +112,7 @@ namespace toku { > toku_cond_t retry_cv; > bool running_retry; > > - void init(void); > + void init(toku_external_mutex_factory_t mutex_factory); > void destroy(void); > }; > > @@ -127,7 +128,8 @@ namespace toku { > void create(lt_create_cb create_cb, > lt_destroy_cb destroy_cb, > lt_escalate_cb escalate_cb, > - void *extra); > + void *extra, > + toku_external_mutex_factory_t mutex_factory_arg); > > void destroy(void); > > @@ -212,6 +214,8 @@ namespace toku { > > omt<locktree *> m_locktree_map; > > + toku_external_mutex_factory_t mutex_factory; > + > // the manager's mutex protects the locktree map > toku_mutex_t m_mutex; > > @@ -275,7 +279,8 @@ namespace toku { > class locktree { > public: > // effect: Creates a locktree > - void create(locktree_manager *mgr, DICTIONARY_ID dict_id, const comparator &cmp); > + void create(locktree_manager *mgr, DICTIONARY_ID dict_id, const comparator &cmp, > + toku_external_mutex_factory_t mutex_factory); > > void destroy(void); > > diff --git a/utilities/transactions/range_locking/locktree/manager.cc b/utilities/transactions/range_locking/locktree/manager.cc > index c31158bb8..47a782565 100644 > --- a/utilities/transactions/range_locking/locktree/manager.cc > +++ b/utilities/transactions/range_locking/locktree/manager.cc > @@ -60,7 +60,12 @@ Copyright (c) 2006, 2015, Percona and/or its affiliates. All rights reserved. > > namespace toku { > > -void locktree_manager::create(lt_create_cb create_cb, lt_destroy_cb destroy_cb, lt_escalate_cb escalate_cb, void *escalate_extra) { > +void locktree_manager::create(lt_create_cb create_cb, > + lt_destroy_cb destroy_cb, > + lt_escalate_cb escalate_cb, > + void *escalate_extra, > + toku_external_mutex_factory_t mutex_factory_arg) { > + mutex_factory= mutex_factory_arg; > m_max_lock_memory = DEFAULT_MAX_LOCK_MEMORY; > m_current_lock_memory = 0; > > @@ -151,7 +156,7 @@ locktree *locktree_manager::get_lt(DICTIONARY_ID dict_id, > locktree *lt = locktree_map_find(dict_id); > if (lt == nullptr) { > XCALLOC(lt); > - lt->create(this, dict_id, cmp); > + lt->create(this, dict_id, cmp, mutex_factory); > > // new locktree created - call the on_create callback > // and put it in the locktree map > @@ -320,7 +325,7 @@ int locktree_manager::iterate_pending_lock_requests(lock_request_iterate_callbac > invariant_zero(r); > > struct lt_lock_request_info *info = lt->get_lock_request_info(); > - toku_mutex_lock(&info->mutex); > + toku_external_mutex_lock(&info->mutex); > > size_t num_requests = info->pending_lock_requests.size(); > for (size_t k = 0; k < num_requests && r == 0; k++) { > @@ -332,7 +337,7 @@ int locktree_manager::iterate_pending_lock_requests(lock_request_iterate_callbac > req->get_conflicting_txnid(), req->get_start_time(), extra); > } > > - toku_mutex_unlock(&info->mutex); > + toku_external_mutex_unlock(&info->mutex); > } > mutex_unlock(); > return r; > @@ -472,10 +477,10 @@ void locktree_manager::get_status(LTM_STATUS statp) { > locktree *lt; > int r = m_locktree_map.fetch(i, &lt); > invariant_zero(r); > - if (toku_mutex_trylock(&lt->m_lock_request_info.mutex) == 0) { > + if (toku_external_mutex_trylock(&lt->m_lock_request_info.mutex) == 0) { > lock_requests_pending += lt->m_lock_request_info.pending_lock_requests.size(); > lt_counters.add(lt->get_lock_request_info()->counters); > - toku_mutex_unlock(&lt->m_lock_request_info.mutex); > + toku_external_mutex_unlock(&lt->m_lock_request_info.mutex); > } > sto_num_eligible += lt->sto_txnid_is_valid_unsafe() ? 1 : 0; > sto_end_early_count += lt->m_sto_end_early_count; > diff --git a/utilities/transactions/range_locking/portability/toku_external_pthread.h b/utilities/transactions/range_locking/portability/toku_external_pthread.h > new file mode 100644 > index 000000000..2482ec507 > --- /dev/null > +++ b/utilities/transactions/range_locking/portability/toku_external_pthread.h > @@ -0,0 +1,81 @@ > +/* > + A wrapper around rocksdb::TransactionDBMutexFactory-provided condition and mutex > + that provides toku_pthread_*-like interface. The functions are named > + > + toku_external_{mutex|cond}_XXX > + > + Lock Tree uses this mutex and condition for interruptible (long) lock waits. > + > + (It also still uses toku_pthread_XXX calls for mutexes/conditions for > + shorter waits on internal objects) > +*/ > + > +#pragma once > + > +#include <pthread.h> > +#include <time.h> > +#include <stdint.h> > + > +#include "toku_portability.h" > + > +#include "rocksdb/utilities/transaction_db.h" > +#include "rocksdb/utilities/transaction_db_mutex.h" > + > +using rocksdb::TransactionDBMutex; > +using rocksdb::TransactionDBCondVar; > + > +typedef std::shared_ptr<rocksdb::TransactionDBMutexFactory> toku_external_mutex_factory_t; > + > +typedef std::shared_ptr<TransactionDBMutex> toku_external_mutex_t; > +typedef std::shared_ptr<TransactionDBCondVar> toku_external_cond_t; > + > +static inline > +void toku_external_cond_init(toku_external_mutex_factory_t mutex_factory, > + toku_external_cond_t *cond) { > + *cond= mutex_factory->AllocateCondVar(); > +} > + > +inline void toku_external_cond_destroy(toku_external_cond_t *cond) { > + cond->reset(); // this will destroy the managed object > +} > + > +inline void toku_external_cond_signal(toku_external_cond_t *cond) { > + (*cond)->Notify(); > +} > + > +inline void toku_external_cond_broadcast(toku_external_cond_t *cond) { > + (*cond)->NotifyAll(); > +} > + > +inline int toku_external_cond_timedwait(toku_external_cond_t *cond, > + toku_external_mutex_t *mutex, > + int32_t timeout_microsec) { > + auto res= (*cond)->WaitFor(*mutex, timeout_microsec); > + if (res.ok()) > + return 0; > + else > + return ETIMEDOUT; > +} > + > +inline void toku_external_mutex_init(toku_external_mutex_factory_t factory, > + toku_external_mutex_t *mutex) { > + *mutex = factory->AllocateMutex(); > +} > + > +inline void toku_external_mutex_lock(toku_external_mutex_t *mutex) { > + (*mutex)->Lock(); > +} > + > +inline int toku_external_mutex_trylock(toku_external_mutex_t *mutex) { > + (*mutex)->Lock(); > + return 0; > +} > + > +inline void toku_external_mutex_unlock(toku_external_mutex_t *mutex) { > + (*mutex)->UnLock(); > +} > + > +inline void toku_external_mutex_destroy(toku_external_mutex_t *mutex) { > + mutex->reset(); // this will destroy the managed object > +} > + > diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc > index 56f3295fd..79164a962 100644 > --- a/utilities/transactions/transaction_lock_mgr.cc > +++ b/utilities/transactions/transaction_lock_mgr.cc > @@ -790,11 +790,6 @@ public: > bool releasing_locks_; > }; > > - > -void RangeLockMgr::KillLockWait(TransactionID txnid) { > - ltm.kill_waiter((void*)txnid); > -} > - > // Get a range lock on [start_key; end_key] range > Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, > uint32_t column_family_id, > @@ -802,7 +797,7 @@ Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, > const rocksdb::Slice &end_key, > bool exclusive) { > toku::lock_request request; > - request.create(); > + request.create(mutex_factory_); > DBT start_key_dbt, end_key_dbt; > > toku_fill_dbt(&start_key_dbt, start_key.data(), start_key.size()); > @@ -810,6 +805,8 @@ Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, > > // Use the txnid as "extra" in the lock_request. Then, KillLockWait() > // will be able to use kill_waiter(txn_id) to kill the wait if needed > + // TODO: KillLockWait is gone and we are no longer using > + // locktree::kill_waiter call. Do we need this anymore? > TransactionID wait_txn_id = txn->GetID(); > > request.set(lt, (TXNID)txn, &start_key_dbt, &end_key_dbt, > @@ -1004,8 +1001,10 @@ int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg, > } > > > -RangeLockMgr::RangeLockMgr(TransactionDB* txn_db) : my_txn_db(txn_db) { > - ltm.create(on_create, on_destroy, on_escalate, NULL); > +RangeLockMgr::RangeLockMgr(TransactionDB* txn_db, > + std::shared_ptr<TransactionDBMutexFactory> mutex_factory) : > + my_txn_db(txn_db), mutex_factory_(mutex_factory) { > + ltm.create(on_create, on_destroy, on_escalate, NULL, mutex_factory_); > lt= nullptr; > cmp_initialized_= false; > } > diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h > index 0c3284519..9f73e5a07 100644 > --- a/utilities/transactions/transaction_lock_mgr.h > +++ b/utilities/transactions/transaction_lock_mgr.h > @@ -215,11 +215,10 @@ class RangeLockMgr : > void UnLock(PessimisticTransaction* txn, uint32_t column_family_id, > const std::string& key, Env* env) override ; > > - RangeLockMgr(TransactionDB* txn_db); > + RangeLockMgr(TransactionDB* txn_db, > + std::shared_ptr<TransactionDBMutexFactory> mutex_factory); > ~RangeLockMgr(); > > - void KillLockWait(TransactionID txnid); > - > int set_max_lock_memory(size_t max_lock_memory) override > { > return ltm.set_max_lock_memory(max_lock_memory); > @@ -251,6 +250,7 @@ class RangeLockMgr : > compare_endpoints_func compare_endpoints; > > TransactionDB* my_txn_db; > + std::shared_ptr<TransactionDBMutexFactory> mutex_factory_; > > static int compare_dbt_endpoints(__toku_db*, void *arg, const DBT *a_key, const DBT *b_key); > > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

2 1

Re: [Maria-developers] [Commits] test
by Roberto Spadim 11 Mar '19

11 Mar '19

noise ignored =) Em seg, 11 de mar de 2019 às 15:11, Faustin Lammler <faustin(a)mariadb.org> escreveu: > Hi, > this is a test, please do not respond. > > And sorry for the noise. > > -- > Faustin Lammler > MariaDB Foundation > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- Roberto Spadim SPAEmpresarial - Software ERP Eng. Automação e Controle

1 0

[Maria-developers] MariaDB-10.4 / Galera and GTIDs - aka we now have two different GTID implementations
by Daniel Black 04 Mar '19

04 Mar '19

Hi, I was excited to come across the Galera-4 release notes and blog. http://galeracluster.com/2019/02/galera-cluster-4-available-for-use-in-the-… It was good to see causal read functions that I'd hoped for in https://jira.mariadb.org/browse/MDEV-10715. WSREP_LAST_WRITTEN_GTID/WSREP_LAST_SEEN_GTID/ WSREP_SYNC_WAIT_UPTO_GTID functions which seemed to mirror existing functions like MASTER_GTID_WAIT and GTID variables. The reason these are different names is because a MariaDB GTID is different from a Galera one. *Having two GTID implementations is insane*. To state the obvious from a user point of view, we've now got two Global Transaction Identification implementations. Application and framework writers like Django/Drupal now have to handle now try to handle consistently with 00000000-0000-0000-0000-000000000000:25 for Galera and 0-1-33 for MariaDB async gtid replication. This is not just a format string difference. The an implementation requires a disparate set of functions to implement when they could of been consolidated. A mixed topology setup could be hidden from the application implementation if the same functions where used. There is/was also a option/proposal that had MariaDB gtid in the protocol save a round trip retrieval of a GTID - quite useful. There is also a bunch of documentation that refers to GTID and its going to absolutely horrible to try to keep the concepts separated as you try to configure a system. An issue like MDEV-10715, the most voted for and watched open issue in JIRA, (and related MDEV-14153), should of had a very priority than this when the implementation from Codership was been merged. This shoehorned in implementation from Codership is a very poor fit. Giving MDEV-10715 a critical priority and a 10.5 target is fiction when there isn't going to be a new galera wsrep api implementation for a number of years. An extra field in the protocol is all it would of taken (comment in MDEV-10715 from 8 months ago). 10.4.3 is out (really an RC?), the galera code got merge in 10.4.2, and the source code of galera-4 isn't even released. Hopefully the tight relationship between MariaDB Corporation and Codership is enough to get this fixed because from an architectural point of view, because the current implementation shouldn't of been merged (contractual arrangement or not). Developers, MariaDB and Codership you've lost track of your user base because the feedback, that is readily available, is ignored. Sadly, Daniel

2 1