- developers - lists.mariadb.org

[Maria-developers] security spring cleaning in MariaDB org on github
by Sergei Golubchik 07 Nov '16

07 Nov '16

Hi, Now when Github has per-user ownership rights and suggests to migrate away from the legacy admin team https://help.github.com/articles/migrating-your-previous-admin-teams-to-the… we're performing some spring cleaning in this area. The legacy admin team (named "Core") is removed. Most of its members lost admin access to the org. Currently only the MariaDB Foundation CEO and few board members (those, who actually have used admin access recently) retained their admin rights. Everyone who was in the Core team should still have write access to repositories, if you've found that it's not the case, please complain asap. If you think you need admin access, please request it (again). Note that - only members of the organization can have it, being in the Developers group is a plus - 2FA is required for all admins (and highly recommended for all other members) Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 5

[Maria-developers] Please review: support for --force-restart in MTR
by Sergey Petrunia 07 Nov '16

07 Nov '16

Hi Sergei, Elena I'm not sure who maintains mysql-test-run.pl but you two seem to have contributed to it in the past, this is why I'm addressing this to you. I'm going to get "MTR v2" to support --force-restart, just like MTR v1 did; http://lists.askmonty.org/pipermail/commits/2016-November/010062.html http://lists.askmonty.org/pipermail/commits/2016-November/010063.html Since I don't really understand this MTRv1 vs MTRv2 change (and why some things are in v1 but not in v2, etc), I wanted to check this change with you. Thanks, Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [MariaDB/server] MDEV-11065 - Compressed binary log (#247)
by Kristian Nielsen 03 Nov '16

03 Nov '16

vinchen <notifications(a)github.com> writes: > The new code is here: > https://github.com/vinchen/server/commits/GCSAdmin-10.2-binlog-compressed2-2 > And added two fixed: > 1.Avoid overflowing buffers in case of corrupt events > 2.Check the compressed algorithm. Looks fine, thanks for fixing this. I have now merged and pushed this to MariaDB 10.2. > > Rows_log_event::write_data_body(). I *think* the reason for that is that > > the > > SQL thread never sees the compressed events (they are uncompressed by the > > IO > > thread), but I would like your confirmation that my understanding is > > correct. > > 3. Did you think about testing that BINLOG statements with compressed > As mentioned above, the compressed events would uncompressed in constructor > in SQL thread. Right, thanks, I understand now. So the BINLOG statements in the output of mysqlbinlog have the uncompressed data. This makes sense, just like the SQL queries are uncompressed before being output by mysqlbinlog. So in fact, one change I suggested is wrong, to accept compressed event types in BINLOG statements. So I reverted this change again, and also added some test cases: https://github.com/MariaDB/server/commit/56a041cde657e5618c519a3c50e8075136… BTW, I think this is recently merged code (from the delayed replication feature) that would not have appeared in your original patch. Thanks for the explanation to help me understand this. I also fixed a .result file - this is a failure that would only show up when running the test suite with --embedded: https://github.com/MariaDB/server/commit/3c0ff6153f75bb8e63c08ff3c828235512… So I think everything should be fine now and this should appear in MariaDB 10.2.3, if I understand correctly. Once again thanks for the patch. It was a pleasure to see a replication addition that was so well done, and with so much attention to detail. - Kristian.

1 0

[Maria-developers] MDEV-11219 main.null fails in buldbot and outside with ps-protocol
by Alexander Barkov 03 Nov '16

03 Nov '16

Hello Sergei, Please review a patch for MDEV-11219. Thanks!

1 0

[Maria-developers] Please review MDEV-10811 Change design from "Item is Type_handler" to "Item has Type_handler"
by Alexander Barkov 03 Nov '16

03 Nov '16

Hello Alexey, Please review a patch for 10.3 for MDEV-10811 Change design from "Item is Type_handler" to "Item has Type_handler Thanks.

1 0

Re: [Maria-developers] [MariaDB/server] Compressed binary log (#247)
by Kristian Nielsen 03 Nov '16

03 Nov '16

GCSAdmin <notifications(a)github.com> writes: > We add new event types to support compress the binlog as follow: > QUERY_COMPRESSED_EVENT, > WRITE_ROWS_COMPRESSED_EVENT_V1, > UPDATE_ROWS_COMPRESSED_EVENT_V1, > DELETE_POWS_COMPRESSED_EVENT_V1, > WRITE_ROWS_COMPRESSED_EVENT, > UPDATE_ROWS_COMPRESSED_EVENT, > DELETE_POWS_COMPRESSED_EVENT > Thanks for the patch! Overall, I'm much impressed with the quality, I see a lot of attention to detail. Below, I have a number of comments and suggestions, but they are all minor for a patch of this complexity. A number of the comments are style fixes or simple changes, and I found it easier to just do the changes to the source for you to review. I hope that is ok. I rebased the series without intermediate merge to simplify review to a single patch. The rebase with my suggested changes on top is here: https://github.com/knielsen/server/commits/GCSAdmin-10.2-binlog-compressed2… Please check the changes I made and let me know if you disagree with anything or I misunderstood something. Changes are mostly: - A bunch of code style (indentation, lines <= 80 chars, clarify comments, and so on). - Change LOG_EVENT_IS_QUERY() etc. from macros to static inline functions. - check_event_type() updated (new code merged into 10.2 recently). - Minor .result file update, also from code merged into 10.2 recently. I also have the following questions/suggestions: 1. I think the code should sanity-check the compressed event header, to check that it does not access outside the event buffer in corrupt events. Eg. if lenlen is 7 and there is less than 7 bytes available in the event. I know that replication code in general is not robust to corrupt events, but it seems best that new code avoids overflowing buffers in case of corrupt event data. 2. There are some places where the compressed event types are not added to switch() or if() statements, mainly related to minimal binlog images, I think: Rows_log_event::read_write_bitmaps_cmp(), Rows_log_event::get_data_size(), Rows_log_event::do_apply_event(), Rows_log_event::write_data_body(). I *think* the reason for that is that the SQL thread never sees the compressed events (they are uncompressed by the IO thread), but I would like your confirmation that my understanding is correct. 3. Did you think about testing that BINLOG statements with compressed events can execute correctly? This happens with mysqlbinlog | mysql, when there are (compressed) row-based events in the binlog. I'm wondering if this could expose compressed events to code that normally runs in the SQL thread and expects to have events uncompressed for it by the IO thread? A few detailed comments on the patch below (most comments are done as changes in the above-linked git branch). Otherwise, after answer to above 3 questions, I think the patch looks good to go into 10.2. - Kristian. ----------------------------------------------------------------------- > +#define BINLOG_COMPRESSED_HEADER_LEN 1 > +#define BINLOG_COMPRESSED_ORIGINAL_LENGTH_MAX_BYTES 4 > +/** > + Compressed Record > + Record Header: 1 Byte > + 0 Bit: Always 1, mean compressed; > + 1-3 Bit: Reversed, compressed algorithm??Always 0, means zlib > + 4-7 Bit: Bytes of "Record Original Length" > + Record Original Length: 1-4 Bytes > + Compressed Buf: I did not understand this. Why "reversed"? It seems to be bits 0-2 that have the bytes of "Record Original Length" and bit 7 that has the '1' bit meaning compression enabled? Maybe this can be clarified. > +int query_event_uncompress(const Format_description_log_event *description_event, bool contain_checksum, > + const char *src, char* buf, ulong buf_size, bool* is_malloc, > + char **dst, ulong *newlen) > +{ > + ulong len = uint4korr(src + EVENT_LEN_OFFSET); > + const char *tmp = src; > + > + DBUG_ASSERT((uchar)src[EVENT_TYPE_OFFSET] == QUERY_COMPRESSED_EVENT); > + > + uint8 common_header_len= description_event->common_header_len; > + uint8 post_header_len= description_event->post_header_len[QUERY_COMPRESSED_EVENT-1]; > + > + tmp += common_header_len; > + > + uint db_len = (uint)tmp[Q_DB_LEN_OFFSET]; > + uint16 status_vars_len= uint2korr(tmp + Q_STATUS_VARS_LEN_OFFSET); > + > + tmp += post_header_len + status_vars_len + db_len + 1; > + > + uint32 un_len = binlog_get_uncompress_len(tmp); > + *newlen = (tmp - src) + un_len; This is one place I would like to see a check on the data in the event. Maybe just that 'len' is larger than 1+(tmp[0]&7). Or maybe simply that len is at least 10, the minimal size for compressed events. > +int row_log_event_uncompress(const Format_description_log_event *description_event, bool contain_checksum, > + const char *src, char* buf, ulong buf_size, bool* is_malloc, > + char **dst, ulong *newlen) > +{ > + uint32 un_len = binlog_get_uncompress_len(tmp); > + *newlen = (tmp - src) + un_len; Another place where I'd like a check against looking outside the event buffer. > +int binlog_buf_uncompress(const char *src, char *dst, uint32 len, uint32 *newlen) > +{ > + if((src[0] & 0x80) == 0) > + { > + return 1; > + } > + > + uint32 lenlen = src[0] & 0x07; > + uLongf buflen = *newlen; > + if(uncompress((Bytef *)dst, &buflen, (const Bytef*)src + 1 + lenlen, len) != Z_OK) Again check on length.

5 12

[Maria-developers] A few character set incompatible changes in 10.2
by Alexander Barkov 02 Nov '16

02 Nov '16

Hello Ian, In 10.2 we made a few changes under terms of these bug reports: MDEV-9874 LOAD XML INFILE does not handle well broken multi-byte characters MDEV-9823 LOAD DATA INFILE silently truncates incomplete byte sequences MDEV-9842 LOAD DATA INFILE does not work well with a TEXT column when using sjis MDEV-9811 LOAD DATA INFILE does not work well with gbk in some cases MDEV-9824 LOAD DATA does not work with multi-byte strings in LINES TERMINATED BY when IGNORE is specified The idea is that the LOAD FILE behavior is now more consistent with INSERT/UPDATE behavior, to store as much data as possible. When some broken byte sequence is found, now LOAD data replaces broken bytes to question marks and keeps loading the value. In older versions LOAD truncated the value on the leftmost broken byte. So suppose I create a file with these bytes: SELECT CONCAT('aaa',0xF09F988E,'bbb') INTO OUTFILE '/tmp/test.txt'; where {{0xF09F988E}} is UTF8MB4 encoding for the character "U+1F60E SMILING FACE WITH SUNGLASSES". and now erroneously load it as a 3-byte utf8: DROP TABLE IF EXISTS t1; CREATE TABLE t1 (a VARCHAR(10) CHARACTER SET utf8); LOAD DATA INFILE '/tmp/test.txt' INTO TABLE t1 CHARACTER SET utf8; SHOW WARNINGS; SELECT * FROM t1; (notice CHARACTER SET utf8 instead of CHARACTER SET utf8mb4 in LOAD). In 5.5 the above script would return: +---------+------+-------------------------------------------------------------------------+ | Level | Code | Message | +---------+------+-------------------------------------------------------------------------+ | Warning | 1366 | Incorrect string value: '\xF0\x9F\x98\x8Ebb...' for column 'a' at row 1 | +---------+------+-------------------------------------------------------------------------+ +------+ | a | +------+ | aaa | +------+ In 10.2 it returns the same warning, but loads more data: +------------+ | a | +------------+ | aaa????bbb | +------------+ Valerii suggests that we document these changes more precisely, mentioning that these are actually incompatible changes! The sad thing is that there is even yet another different behavior in 10.0.26: MDEV-11217 Regression: LOAD DATA INFILE started to fail with an error We're currently thinking what to do with that.

1 0

[Maria-developers] Replication compatibility MySQL/MariaDB
by Daniel Black 02 Nov '16

02 Nov '16

Folks, At the MariaDB Developer meetup there was some discussion about the limits of compatibility especially regarding replication. I've started a section on this page: https://mariadb.com/kb/en/mariadb/mariadb-vs-mysql-compatibility/ If you can identify what can/cannot work in various replication modes between MySQL and MariaDB please document it on this page. If you cannot edit this for some reason reply to the MariaDB Discuss list and eventually the results can be correlated and the page can be updated. As usual other material changes to improving the KB documentation is welcome. Daniel

1 0

[Maria-developers] MDEV-7660 MySQL WL#6671 "Improve scalability by not using thr_lock.c locks for InnoDB tables"
by Sergei Golubchik 29 Oct '16

29 Oct '16

Hi, Sergey, I think it's fine. I had a few questions though, see below: > commit 47aa5f9 > Author: Sergey Vojtovich <svoj(a)mariadb.org> > Date: Fri May 6 13:44:07 2016 +0400 > > MDEV-7660 - MySQL WL#6671 "Improve scalability by not using thr_lock.c locks > for InnoDB tables" > > Don't use thr_lock.c locks for InnoDB tables. > > Let HANDLER READ call external_lock() even if SE is not going to be locked by > THR_LOCK. This fixes at least main.implicit_commit failure. > > Removed tests for BUG#45143 and BUG#55930 which cover InnoDB + THR_LOCK. To > operate properly these tests require code flow to go through THR_LOCK debug > sync points, which is not the case after this patch. These tests are removed > by WL#6671 as well. An alternative is to port them to different storage engine. > > For the very same reason partition_debug_sync test was adjusted to use MyISAM. > > diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc > index e8ade81..76107ae 100644 > --- a/sql/sql_handler.cc > +++ b/sql/sql_handler.cc > @@ -752,11 +752,12 @@ bool mysql_ha_read(THD *thd, TABLE_LIST *tables, > tables->table= table; // This is used by fix_fields > table->pos_in_table_list= tables; > > - if (handler->lock->lock_count > 0) > + if (handler->lock->table_count > 0) > { > int lock_error; > > - handler->lock->locks[0]->type= handler->lock->locks[0]->org_type; > + if (handler->lock->lock_count > 0) > + handler->lock->locks[0]->type= handler->lock->locks[0]->org_type; I don't understand this code in mysql_ha_read() at all :( even before your changes > /* save open_tables state */ > TABLE* backup_open_tables= thd->open_tables; > diff --git a/storage/xtradb/handler/ha_innodb.h b/storage/xtradb/handler/ha_innodb.h > index 2027a59..efb8120 100644 > --- a/storage/xtradb/handler/ha_innodb.h > +++ b/storage/xtradb/handler/ha_innodb.h > @@ -218,6 +218,7 @@ class ha_innobase: public handler > bool can_switch_engines(); > uint referenced_by_foreign_key(); > void free_foreign_key_create_info(char* str); > + uint lock_count(void) const; > THR_LOCK_DATA **store_lock(THD *thd, THR_LOCK_DATA **to, > enum thr_lock_type lock_type); > void init_table_handle_for_HANDLER(); > > commit 5645626 > Author: Sergey Vojtovich <svoj(a)mariadb.org> > Date: Tue May 24 12:25:56 2016 +0400 > > MDEV-7660 - MySQL WL#6671 "Improve scalability by not using thr_lock.c locks > for InnoDB tables" > > - InnoDB now acquires shared lock for HANDLER ... READ Why? > - LOCK TABLES now disables autocommit implicitely > - UNLOCK TABLES now re-enables autocommit implicitely if it was disabled by > LOCK TABLES > - adjusted test cases to this new behavior > > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index 3091bd6..7d39484 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -2824,6 +2824,8 @@ Locked_tables_list::unlock_locked_tables(THD *thd) > request for metadata locks and TABLE_LIST elements. > */ > reset(); > + if (thd->variables.option_bits & OPTION_AUTOCOMMIT) > + thd->variables.option_bits&= ~(OPTION_NOT_AUTOCOMMIT); 1. Was it possible - before your change - for OPTION_AUTOCOMMIT and OPTION_NOT_AUTOCOMMIT to be out of sync? 2. What if someone changes @@autocommit under LOCK TABLES? Do you have a test for that? 3. Do you need to set SERVER_STATUS_AUTOCOMMIT here? > } > > > diff --git a/mysql-test/t/innodb_mysql_lock.test b/mysql-test/t/innodb_mysql_lock.test > index cb57c09..85ba418 100644 > --- a/mysql-test/t/innodb_mysql_lock.test > +++ b/mysql-test/t/innodb_mysql_lock.test > @@ -150,14 +150,16 @@ let $wait_condition= > --source include/wait_condition.inc > LOCK TABLES t1 READ; > SELECT release_lock('bug42147_lock'); > +let $wait_condition= > + SELECT COUNT(*) > 0 FROM information_schema.processlist > + WHERE state = 'executing' > + AND info = 'INSERT INTO t1 SELECT get_lock(\'bug42147_lock\', 60)'; > +--source include/wait_condition.inc > +UNLOCK TABLES; I don't understand the original test case. But after your changes it actually makes sense :) > > connection default; > --reap > > -connection con2; > -UNLOCK TABLES; > - > -connection default; > disconnect con2; > DROP TABLE t1; > > diff --git a/mysql-test/r/partition_explicit_prune.result b/mysql-test/r/partition_explicit_prune.result > index 765803d..7b9c53d 100644 > --- a/mysql-test/r/partition_explicit_prune.result > +++ b/mysql-test/r/partition_explicit_prune.result > @@ -281,7 +281,7 @@ UNLOCK TABLES; > SELECT * FROM INFORMATION_SCHEMA.SESSION_STATUS > WHERE VARIABLE_NAME LIKE 'HANDLER_%' AND VARIABLE_VALUE > 0; > VARIABLE_NAME VARIABLE_VALUE > -HANDLER_COMMIT 2 > +HANDLER_COMMIT 3 why is that? > HANDLER_READ_RND_NEXT 52 > HANDLER_TMP_WRITE 72 > HANDLER_WRITE 2 > diff --git a/mysql-test/suite/handler/handler.inc b/mysql-test/suite/handler/handler.inc > index b1e881f..8cad6a5 100644 > --- a/mysql-test/suite/handler/handler.inc > +++ b/mysql-test/suite/handler/handler.inc > @@ -1091,6 +1091,12 @@ connection default; > --reap > drop table t2; > > +# This test expects "handler t1 read a next" to get blocked on table level > +# lock so that further "drop table t1" can break the lock and close handler. > +# This notification mechanism doesn't work with InnoDB since it bypasses > +# table level locks. what happens for InnoDB then? I'd expect "handler t1 read a next" to get blocked inside the InnoDB. what does then "drop table t1" do? > +if ($engine_type != 'InnoDB') > +{ > --echo # > --echo # Bug #46224 HANDLER statements within a transaction might > --echo # lead to deadlocks Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 8

[Maria-developers] Patch for unaligned word access in CONNECT storage engine
by Kristian Nielsen 28 Oct '16

28 Oct '16

Who should be contacted about issues in the CONNECT storage engine? The attached patch is from Debian Bug#838914 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=838914 Apparently the code does direct unaligned accesses of word data. This works fine on the x86 architecture, but on some other architectures (like MIPS), it causes a bus error. In other places in the server code, the similar issue is handled correctly with uint4korr() and similar macros (though these also deal with byte order). I think the patch (or something similar) is good and should be upstreamed. But I am not sure how the CONNECT storage engine is maintained - should this go directly into MariaDB? If there is an upstream maintained CONNECT storage engine, probably it should preferably go there first? - Kristian.g

2 3