- developers - lists.mariadb.org

Re: [Maria-developers] c850799967c: MDEV-10963 Fragmented BINLOG query
by Sergei Golubchik 23 Oct '18

23 Oct '18

Hi, Andrei! First, when I run it I got: Warning: 112 bytes lost at 0x7f212f26a670, allocated by T@0 at sql/sql_binlog.cc:68, sql/sql_binlog.cc:181, sql/sql_parse.cc:5625, sql/sql_parse.cc:7465, sql/sql_parse.cc:1497, sql/sql_parse.cc:1124, sql/sql_connect.cc:1330, sql/sql_connect.cc:1243 so you have a memory leak with your thd->lex->comment.str Second, BINGLOG CONCAT() is totally possible, inventing a new keyword defeats the whole point of using a familiar and intuitive syntax. I've attached a simple patch that implements it. On the other hand, I just thought that CONCAT() syntax might be not so intuitive after all, as - strictly speaking - a result of CONCAT cannot be larger than max_allowed_packet. So it might be confusing to use CONCAT here. May be just use your idea of BINLOG @a, @b; with no CONCAT or anything else at all? Also, I'm thinking whether BINLOG (in this last form) could automatically clear its variables @a and @b. Without an explicit SET @a=NULL. It's a bit weird to have such a side effect. But it'll help to save memory - the event size it over 3GB, right? So you'll have it split in two variables. Then you create a concatenated copy (it's over 3GB more), then you decode it (already more than 9GB in memory), and then it's executed where copies of huge blob values are created again, may be more than once. Freeing @a and @b and the concatenated copy as soon as possible will save over 6GB in RAM usage. Note, we cannot auto-reset variables in the syntax with CONCAT(), DEFRAGMENT() or whatever, because functions cannot do it to their arguments. On Oct 18, Andrei Elkin wrote: > revision-id: c850799967c561d08242987269d94af6ae4c7c5e (mariadb-10.1.35-59-gc850799967c) > parent(s): d3a8b5aa9cee24a6397662e30df2e915f45460e0 > author: Andrei Elkin <andrei.elkin(a)mariadb.com> > committer: Andrei Elkin <andrei.elkin(a)mariadb.com> > timestamp: 2018-09-21 18:48:06 +0300 > message: > > MDEV-10963 Fragmented BINLOG query > > The problem was originally stated in > http://bugs.mysql.com/bug.php?id=82212 > The size of an base64-encoded Rows_log_event exceeds its > vanilla byte representation in 4/3 times. > When a binlogged event size is about 1GB mysqlbinlog generates > a BINLOG query that can't be send out due to its size. > > It is fixed with fragmenting the BINLOG argument C-string into > (approximate) halves when the base64 encoded event is over 1GB size. > The mysqlbinlog in such case puts out > > SET @binlog_fragment_0='base64-encoded-fragment_0'; > SET @binlog_fragment_1='base64-encoded-fragment_1'; > BINLOG DEFRAGMENT(@binlog_fragment_0, @binlog_fragment_1); > > to represent a big BINLOG 'base64-encoded-"total"'. > Two more statements are composed to promptly release memory > SET @binlog_fragment_0=NULL; > SET @binlog_fragment_1=NULL; > > The 2 fragments are enough, though the client and server still may > need to tweak their @@max_allowed_packet to satisfy to the fragment > size (which they would have to do anyway with greater number of > fragments, should that be desired). > > On the lower level the following changes are made: > > Log_event::print_base64() > remains to call encoder and store the encoded data into a cache but > now *without* doing any formatting. The latter is left for time > when the cache is copied to an output file (e.g mysqlbinlog output). > No formatting behavior is also reflected by the change in the meaning > of the last argument which specifies whether to cache the encoded data. > > my_b_copy_to_file() > is turned into my_b_copy_to_file_frag() > which accepts format specifier arguments to build a proper syntax BINLOG > query in both the fragmented (n_frag > 1) and non-fragmented (n_frag == 1) > cases. > > Rows_log_event::print_helper() > Takes decision whether to fragment, prepares respective format specifiers > and invokes the cache-to-file copying function, which is now > > copy_cache_frag_to_file_and_reinit() > replaces original copy_event_cache_to_file_and_reinit() to pass > extra arguments to my_b_copy_to_file() successor of > > my_b_copy_to_file_frag() > replaces the former pure copier not to also conduct wrapping the encoded > data per format specifiers. > With its 'n_frag' argument as 1 > and the rest or args NULL works as the original function. > > diff --git a/mysql-test/suite/binlog/t/binlog_mysqlbinlog_row_frag.test b/mysql-test/suite/binlog/t/binlog_mysqlbinlog_row_frag.test > new file mode 100644 > index 00000000000..5318f8acec6 > --- /dev/null > +++ b/mysql-test/suite/binlog/t/binlog_mysqlbinlog_row_frag.test > @@ -0,0 +1,43 @@ > +--source include/have_debug.inc > +--source include/have_log_bin.inc > +--source include/have_binlog_format_row.inc > + > +--let $MYSQLD_DATADIR= `select @@datadir` > +--let $max_size=1024 > + > +CREATE TABLE t (a TEXT); > +# events of interest are guaranteed to stay in 000001 log > +RESET MASTER; > +--eval INSERT INTO t SET a=repeat('a', $max_size) > +SELECT a from t into @a; > +FLUSH LOGS; > +DELETE FROM t; > + > +--exec $MYSQL_BINLOG --debug-binlog-row-event-max-encoded-size=256 $MYSQLD_DATADIR/master-bin.000001 > $MYSQLTEST_VARDIR/tmp/mysqlbinlog.sql > + > +--exec $MYSQL test < $MYSQLTEST_VARDIR/tmp/mysqlbinlog.sql how do you know the event was split, if you aren't looking into mysqlbinlog.sql file? may be your code doesn't work at all, or it was removed by some incorrect merge? the test will pass anyway... > + > +SELECT a LIKE @a as 'true' FROM t; > +SELECT @binlog_fragment_0, @binlog_fragment_1 as 'NULL'; > + > +# improper syntax error > +--echo BINLOG number-of-fragments must be exactly two > +--error ER_PARSE_ERROR > +BINLOG DEFRAGMENT(@binlog_fragment); > +--error ER_PARSE_ERROR > +BINLOG DEFRAGMENT(@binlog_fragment, @binlog_fragment, @binlog_fragment); > + > +# corrupted fragments error check (to the expected error code notice, > +# the same error code occurs in a similar unfragmented case) > +SET @binlog_fragment_0='012345'; > +SET @binlog_fragment_1='012345'; > +--error ER_SYNTAX_ERROR > +BINLOG DEFRAGMENT(@binlog_fragment_0, @binlog_fragment_1); > + > +# Not existing fragment is not allowed > +SET @binlog_fragment_0='012345'; > +--error ER_BASE64_DECODE_ERROR > +BINLOG DEFRAGMENT(@binlog_fragment_0, @binlog_fragment_not_exist); > + > +--echo # Cleanup > +DROP TABLE t; > diff --git a/mysys/mf_iocache2.c b/mysys/mf_iocache2.c > index 2499094037d..6b7ff8a7568 100644 > --- a/mysys/mf_iocache2.c > +++ b/mysys/mf_iocache2.c > @@ -22,13 +22,23 @@ > #include <stdarg.h> > #include <m_ctype.h> > > -/* > +/** > Copy contents of an IO_CACHE to a file. > > SYNOPSIS > - my_b_copy_to_file() > - cache IO_CACHE to copy from > - file File to copy to > + my_b_copy_to_file_frag > + > + cache IO_CACHE to copy from > + file File to copy to > + n_frag # of fragments > + > + Other arguments represent format strings to enable wrapping > + of the fragments and their total, including > + > + before_frag before a fragment > + after_frag after a fragment > + after_last_frag after all the fragments > + final_per_frag in the end per each fragment if you edit a comment, change it to Doxygen syntax. Especially, as you start it with Doxygen's /**, it is simply invalid to have SYNOPSIS and DESCRIPTION inside. > > DESCRIPTION > Copy the contents of the cache to the file. The cache will be > @@ -38,33 +48,120 @@ > If a failure to write fully occurs, the cache is only copied > partially. > > - TODO > - Make this function solid by handling partial reads from the cache > - in a correct manner: it should be atomic. > + The copying is made in so many steps as the number of fragments as > + specified by the parameter 'n_frag'. Each step is wrapped with > + writing to the file 'before_frag' and 'after_frag' formated > + strings, unless the parameters are NULL. In the end, optionally, > + first 'after_last_frag' string is appended to 'file' followed by > + 'final_per_frag' per each fragment. > + final item. > > RETURN VALUE > 0 All OK > 1 An error occurred > + > + TODO > + Make this function solid by handling partial reads from the cache > + in a correct manner: it should be atomic. > */ > int > -my_b_copy_to_file(IO_CACHE *cache, FILE *file) > +my_b_copy_to_file_frag(IO_CACHE *cache, FILE *file, > + uint n_frag, > + const char* before_frag, > + const char* after_frag, > + const char* after_last, > + const char* final_per_frag, > + char* buf) > { > - size_t bytes_in_cache; > - DBUG_ENTER("my_b_copy_to_file"); > + size_t bytes_in_cache; // block, may have short size in the last one > + size_t written_off_last_block; // consumed part of the block by last fragment > + size_t total_size= my_b_tell(cache); > + size_t frag_size= total_size / n_frag + 1; > + size_t total_written= 0; > + size_t frag_written; // bytes collected in the current fragment > + uint i; > + > + DBUG_ENTER("my_b_copy_to_file_frag"); > + > + DBUG_ASSERT(cache->type == WRITE_CACHE); > > /* Reinit the cache to read from the beginning of the cache */ > if (reinit_io_cache(cache, READ_CACHE, 0L, FALSE, FALSE)) > DBUG_RETURN(1); > - bytes_in_cache= my_b_bytes_in_cache(cache); > - do > - { > - if (my_fwrite(file, cache->read_pos, bytes_in_cache, > - MYF(MY_WME | MY_NABP)) == (size_t) -1) > - DBUG_RETURN(1); > - } while ((bytes_in_cache= my_b_fill(cache))); > - if(cache->error == -1) > - DBUG_RETURN(1); > - DBUG_RETURN(0); > + > + for (i= 0, written_off_last_block= 0, bytes_in_cache= my_b_bytes_in_cache(cache); > + i < n_frag; > + i++, total_written += frag_written) > + { > + frag_written= 0; > + if (before_frag) > + { > + sprintf(buf, before_frag, i); > + my_fwrite(file, (uchar*) buf, strlen(buf), MYF(MY_WME | MY_NABP)); > + } > + do > + { > + /* > + Either the current block is the last (L) in making the > + current fragment (and possibly has some extra not to fit (LG) into > + the fragment), or (I) the current (whole then) block is > + intermediate. > + */ > + size_t block_to_write= (frag_written + bytes_in_cache >= frag_size) ? > + frag_size - frag_written : bytes_in_cache; > + > + DBUG_ASSERT(n_frag != 1 || > + (block_to_write == bytes_in_cache && > + written_off_last_block == 0)); > + > + if (my_fwrite(file, cache->read_pos + written_off_last_block, > + block_to_write, > + MYF(MY_WME | MY_NABP)) == (size_t) -1) > + /* no cache->error is set here */ > + DBUG_RETURN(1); > + > + frag_written += block_to_write; > + if (frag_written == frag_size) // (L) > + { > + DBUG_ASSERT(block_to_write <= bytes_in_cache); > + written_off_last_block= block_to_write; > + bytes_in_cache -= written_off_last_block; // (LG) when bytes>0 > + /* > + Nothing should be left in cache at the end of the > + last fragment construction. > + */ > + DBUG_ASSERT(i != n_frag - 1 || bytes_in_cache == 0); > + > + break; > + } > + else > + { > + written_off_last_block= 0; // (I) > + } > + } while ((bytes_in_cache= my_b_fill(cache))); > + > + if (after_frag) > + { > + sprintf(buf, after_frag, NULL); > + my_fwrite(file, (uchar*) buf, strlen(buf), MYF(MY_WME | MY_NABP)); > + } > + } > + > + DBUG_ASSERT(total_written == total_size); // output == input > + > + if (after_last) > + { > + sprintf(buf, after_last, n_frag); > + my_fwrite(file, (uchar*) buf, strlen(buf), MYF(MY_WME | MY_NABP)); > + } > + > + for (i= 0; final_per_frag && i < n_frag ; i++) > + { > + sprintf(buf, final_per_frag, i); > + my_fwrite(file, (uchar*) buf, strlen(buf), MYF(MY_WME | MY_NABP)); > + } > + > + DBUG_RETURN(cache->error == -1); this is a big and very specialized function with non-standard indentation. Please, move it from mysys to mysqlbinlog.cc and fix the indentation. > } > > > diff --git a/sql/log_event.cc b/sql/log_event.cc > index e1912ad4620..0c39200148f 100644 > --- a/sql/log_event.cc > +++ b/sql/log_event.cc > @@ -10474,12 +10482,108 @@ void Rows_log_event::pack_info(Protocol *protocol) > #endif > > #ifdef MYSQL_CLIENT > +void copy_cache_to_file_wrapped(FILE *file, > + PRINT_EVENT_INFO *print_event_info, > + IO_CACHE *body, > + bool do_print_encoded) > +{ > + uint n_frag= 1; > + const char* before_frag= NULL; > + char* after_frag= NULL; > + char* after_last= NULL; > + char* final_per_frag= NULL; > + /* > + 2 fragments can always represent near 1GB row-based > + base64-encoded event as two strings each of size less than > + max(max_allowed_packet). Greater number of fragments does not > + save from potential need to tweak (increase) @@max_allowed_packet > + before to process the fragments. So 2 is safe and enough. > + */ > + const char fmt_last_frag2[]= > + "\nBINLOG DEFRAGMENT(@binlog_fragment_0, @binlog_fragment_1)%s\n"; > + const char fmt_last_per_frag[]= "\nSET @binlog_fragment_%%d = NULL%s\n"; this is completely unnecessary "generalization", print simply "SET @binlog_fragment_0=NULL,binlog_fragment_1=NULL%s\n" in particular, as your "BINLOG DEFRAGMENT" statement is not generalized. (or to the cleanup inside BINLOG, as discussed above). > + const char fmt_before_frag[]= "\nSET @binlog_fragment_%d ='\n"; > + /* > + Buffer to pass to copy_cache_frag_to_file_and_reinit to > + compute formatted strings according to specifiers. > + The sizes may depend on an actual fragment number size in terms of decimal > + signs so its maximum is estimated (not precisely yet safely) below. > + */ > + char buf[(sizeof(fmt_last_frag2) + sizeof(fmt_last_per_frag)) > + + ((sizeof(n_frag) * 8)/3 + 1) // decimal index > + + sizeof(print_event_info->delimiter) + 3]; // delim, \n and 0. > + > + if (do_print_encoded) > + { > + after_frag= (char*) my_malloc(sizeof(buf), MYF(MY_WME)); > + sprintf(after_frag, "'%s\n", print_event_info->delimiter); > + if (my_b_tell(body) > opt_binlog_rows_event_max_encoded_size) > + n_frag= 2; > + if (n_frag > 1) > + { > + before_frag= fmt_before_frag; > + after_last= (char*) my_malloc(sizeof(buf), MYF(MY_WME)); > + sprintf(after_last, fmt_last_frag2, (char*) print_event_info->delimiter); > + final_per_frag= (char*) my_malloc(sizeof(buf), MYF(MY_WME)); > + sprintf(final_per_frag, fmt_last_per_frag, > + (char*) print_event_info->delimiter); > + } > + else > + { > + before_frag= "\nBINLOG '\n"; > + } > + } > + if (copy_cache_frag_to_file_and_reinit(body, file, n_frag, > + before_frag, after_frag, > + after_last, final_per_frag, buf)) 1. get rid of this copy_cache_frag_to_file_and_reinit function, it's not helping. 2. instead of pushing all this complex logic of printing fragments with before and after and in-between and start and final, it'd be *much* simpler to do it here and just extend my_b_copy_to_file() to print at most N bytes. > + { > + body->error= -1; > + goto err; > + } > + > +err: > + my_free(after_frag); > + my_free(after_last); > + my_free(final_per_frag); > +} > + > +/* > + The function invokes base64 encoder to run on the current > + event string and store the result into two caches. > + When the event ends the current statement the caches are is copied into > + the argument file. > + Copying is also concerned how to wrap the event, specifically to produce > + a valid SQL syntax. > + When the encoded data size is within max(MAX_ALLOWED_PACKET) > + a regular BINLOG query is composed. Otherwise it is build as fragmented > + > + SET @binlog_fragment_0='...'; > + SET @binlog_fragment_1='...'; > + BINLOG DEFRAGMENT(@binlog_fragment_0, @binlog_fragment_1); > + > + where fragments are represented by a sequence of "indexed" user > + variables. > + Two more statements are composed as well > + > + SET @binlog_fragment_0=NULL; > + SET @binlog_fragment_1=NULL; > + > + to promptly release memory. > + > + NOTE. > + If any changes made don't forget to duplicate them to > + Old_rows_log_event as long as it's supported. > +*/ > void Rows_log_event::print_helper(FILE *file, > PRINT_EVENT_INFO *print_event_info, > char const *const name) > { > IO_CACHE *const head= &print_event_info->head_cache; > IO_CACHE *const body= &print_event_info->body_cache; > + bool do_print_encoded= > + print_event_info->base64_output_mode != BASE64_OUTPUT_DECODE_ROWS && > + !print_event_info->short_form; > + > if (!print_event_info->short_form) > { > bool const last_stmt_event= get_flags(STMT_END_F); > diff --git a/sql/sql_binlog.cc b/sql/sql_binlog.cc > index 91cf038907e..b15d72e036a 100644 > --- a/sql/sql_binlog.cc > +++ b/sql/sql_binlog.cc > @@ -28,6 +28,73 @@ > // START_EVENT_V3, > // Log_event_type, > // Log_event > + > +/* > + Copy fragments into the standard placeholder thd->lex->comment.str. > + > + Compute the size of the (still) encoded total, > + allocate and then copy fragments one after another. > + The size can exceed max(max_allowed_packet) which is not a > + problem as no String instance is created off this char array. > + > + Return 0 at success, -1 otherwise. > +*/ > +int binlog_defragment(THD *thd) > +{ > + LEX_STRING *curr_frag_name; > + > + thd->lex->comment.length= 0; > + thd->lex->comment.str= NULL; > + /* compute the total size */ > + for (uint i= 0; i < thd->lex->fragmented_binlog_event.n_frag; i++) > + { > + user_var_entry *entry; > + > + curr_frag_name= &thd->lex->fragmented_binlog_event.frag_name[i]; > + entry= > + (user_var_entry*) my_hash_search(&thd->user_vars, > + (uchar*) curr_frag_name->str, > + curr_frag_name->length); > + if (!entry || entry->type != STRING_RESULT) > + { > + my_printf_error(ER_BASE64_DECODE_ERROR, > + "%s: BINLOG fragment user " > + "variable '%s' has unexpectedly no value", MYF(0), > + ER_THD(thd, ER_BASE64_DECODE_ERROR), curr_frag_name->str); > + return -1; > + } > + thd->lex->comment.length += entry->length; > + } > + thd->lex->comment.str= // to be freed by the caller > + (char *) my_malloc(thd->lex->comment.length, MYF(MY_WME)); > + if (!thd->lex->comment.str) > + { > + my_error(ER_OUTOFMEMORY, MYF(ME_FATALERROR), 1); > + return -1; > + } > + > + /* fragments are merged into allocated buf */ > + size_t gathered_length= 0; > + for (uint i= 0; i < thd->lex->fragmented_binlog_event.n_frag; i++) > + { > + user_var_entry *entry; > + > + curr_frag_name= &thd->lex->fragmented_binlog_event.frag_name[i]; > + entry= > + (user_var_entry*) my_hash_search(&thd->user_vars, > + (uchar*) curr_frag_name->str, > + curr_frag_name->length); > + memcpy(thd->lex->comment.str + gathered_length, > + entry->value, entry->length); > + > + gathered_length += entry->length; > + } > + DBUG_ASSERT(gathered_length == thd->lex->comment.length); ok. 1. BINLOG uses LEX::comment to avoid increasing sizeof(LEX) for a rarely used statement. So don't increase it for a much-more-rarely used event overlow case. That is, remove fragmented_binlog_event. You can use a pair of LEX_STRING's in LEX (e.g. comment/ident) or (less hackish) List<LEX_STRING>, which are plenty in LEX. 2. Don't look up variables twice, remember them in the first loop. You can unroll it too, there're only two variables anyway. E.g. the second loop can be replaced by just memcpy(thd->lex->comment.str, var[0]->value, var[0]->length); memcpy(thd->lex->comment.str + var[0]->length, var[1]->value, var[1]->length); or even shorter, if you start your function with LEX_STRING *out= &thd->lex->comment; 3. Not "unexpectedly no value". ER_WRONG_TYPE_FOR_VAR looks more appropriate there. > + > + return 0; > +} > + > + > /** > Execute a BINLOG statement. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 4

[Maria-developers] MDEV-13912
by Michal Schorm 22 Oct '18

22 Oct '18

Hello, can I get some status update on MDEV-13912 please? It is still an issue for the customer on my side and I'd love to see fixed it in 5.5.62 release. Michal -- Michal Schorm Associate Software Engineer Core Services - Databases Team Red Hat

2 2

Re: [Maria-developers] cf9bafc: MDEV-14815 - Server crash or AddressSanitizer errors or valgrind warnings
by Sergei Golubchik 19 Oct '18

19 Oct '18

Hi, Sergey! On Oct 19, Sergey Vojtovich wrote: > revision-id: cf9bafc8cb0943c914d118525888140ea15e6b55 (mariadb-10.0.36-50-gcf9bafc) > parent(s): 8e716138cef2d9be603cdf71701d82bcb72dfd69 > author: Sergey Vojtovich > committer: Sergey Vojtovich > timestamp: 2018-10-18 23:50:50 +0400 > message: > > MDEV-14815 - Server crash or AddressSanitizer errors or valgrind warnings > in thr_lock / has_old_lock upon FLUSH TABLES > > Explicit partition access of partitioned MEMORY table under LOCK TABLES > may cause subsequent statements to crash the server, deadlock, trigger > valgrind warnings or ASAN errors. Freed memory was being used due to > incorrect cleanup. > > At least MyISAM and InnoDB don't seem to be affected, since their > THR_LOCK structures don't survive FLUSH TABLES. MEMORY keeps table shared > data (including THR_LOCK) even if there're no open instances. > > There's partition_info::lock_partitions bitmap, which holds bits of > partitions allowed to be accessed after pruning. This bitmap is > updated for each individual statement. > > This bitmap was abused in ha_partition::store_lock() such that when we > need to unlock a table, locked by LOCK TABLES, only locks for partitions > that were accessed by previous statement were released. > > Eventually FLUSH TABLES frees THR_LOCK_DATA objects, which are still > linked into THR_LOCK lists. When such THR_LOCK gets reused we end up with > freed memory access. > > Fixed by using ha_partition::m_locked_partitions bitmap similarly to > ha_partition::external_lock(). Thanks, good comment. > Some unused code removed. Generally, it's better to do that in a separate commit. It's very easy to do with git citool, and doesn't requite any advance planning, you just edit whatever you like and then run 'git citool' twice, selecting what lines you want to commit each time. > diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc > index 0488ebf..02de92a 100644 > --- a/sql/ha_partition.cc > +++ b/sql/ha_partition.cc > @@ -3907,9 +3907,14 @@ THR_LOCK_DATA **ha_partition::store_lock(THD *thd, > } > else > { > - for (i= bitmap_get_first_set(&(m_part_info->lock_partitions)); > + MY_BITMAP *used_partitions= lock_type == TL_UNLOCK || > + lock_type == TL_IGNORE ? why TL_IGNORE too? external_lock only checks for TL_UNLOCK. > + &m_locked_partitions : > + &m_part_info->lock_partitions; > + > + for (i= bitmap_get_first_set(used_partitions); > i < m_tot_parts; > - i= bitmap_get_next_set(&m_part_info->lock_partitions, i)) > + i= bitmap_get_next_set(used_partitions, i)) > { > DBUG_PRINT("info", ("store lock %d iteration", i)); > to= m_file[i]->store_lock(thd, to, lock_type); Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] 7ce7184cf0e: MDEV-17133 dump thread reads from a past position
by Sergei Golubchik 19 Oct '18

19 Oct '18

Hi, Andrei! ok to push. But please add a meaningful commit comment. Thanks! On Oct 19, Andrei Elkin wrote: > revision-id: 7ce7184cf0e354a1c9114bb31df1fb63b1fbbcc7 (mariadb-10.1.35-59-g7ce7184cf0e) > parent(s): d3a8b5aa9cee24a6397662e30df2e915f45460e0 > author: Andrei Elkin <andrei.elkin(a)mariadb.com> > committer: Andrei Elkin <andrei.elkin(a)mariadb.com> > timestamp: 2018-09-18 23:21:18 +0300 > message: > > MDEV-17133 dump thread reads from a past position > > --- > mysys/mf_iocache.c | 8 +++-- > unittest/sql/mf_iocache-t.cc | 74 +++++++++++++++++++++++++++++++++++++++++++- > 2 files changed, 79 insertions(+), 3 deletions(-) > > diff --git a/mysys/mf_iocache.c b/mysys/mf_iocache.c > index 56b1ae3fc6e..8f829a8c9c2 100644 > --- a/mysys/mf_iocache.c > +++ b/mysys/mf_iocache.c > @@ -563,7 +563,7 @@ int _my_b_write(IO_CACHE *info, const uchar *Buffer, size_t Count) > > int _my_b_cache_read(IO_CACHE *info, uchar *Buffer, size_t Count) > { > - size_t length, diff_length, left_length= 0, max_length; > + size_t length= 0, diff_length, left_length= 0, max_length; > my_off_t pos_in_file; > DBUG_ENTER("_my_b_cache_read"); > > @@ -668,7 +668,11 @@ int _my_b_cache_read(IO_CACHE *info, uchar *Buffer, size_t Count) > else > { > info->error= 0; > - DBUG_RETURN(0); /* EOF */ > + if (length == 0) > + { > + DBUG_RETURN(0); // EOF > + } > + length= 0; > } > } > else if ((length= mysql_file_read(info->file,info->buffer, max_length, Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Correct parsing of BINLOG 'XXX' with comment inside
by Sachin Setia 19 Oct '18

19 Oct '18

Hi Bar, Currently if we have generated the sql file using mysqlbinlog -v and if we have big statement binlog , then mysqlbinlog add comment in between of Binlog 'XXX' statement , but unfortunately base64_decode does not understands it, and it through error since # is not a base64 char This patches solves this. There are 2 patches one for 5.5 and 2nd for 10.0 and above Please review it.

4 7

Re: [Maria-developers] 9b453c1ebf4: MDEV-10963 Fragmented BINLOG query
by Sergei Golubchik 18 Oct '18

18 Oct '18

Hi, Andrei! On Sep 07, andrei.elkin(a)pp.inet.fi wrote: > revision-id: 9b453c1ebf47a20ea1436de3284f8810d0e64c4c (mariadb-10.1.34-8-g9b453c1ebf4) > parent(s): c09a8b5b36edb494e2bcc93074c06e26cd9f2b92 > author: Andrei Elkin > committer: Andrei Elkin > timestamp: 2018-09-07 20:36:16 +0300 > message: > > MDEV-10963 Fragmented BINLOG query > > The problem was originally stated in > http://bugs.mysql.com/bug.php?id=82212 > The size of an base64-encoded Rows_log_event exceeds its > vanilla byte representation in 4/3 times. > When a binlogged event size is about 1GB mysqlbinlog generates > a BINLOG query that can't be send out due to its size. > > It is fixed with fragmenting the BINLOG argument C-string into > (approximate) halves when the base64 encoded event is over 1GB size. > The mysqlbinlog in such case puts out > > SET @binlog_fragment_0='base64-encoded-fragment_0'; > SET @binlog_fragment_1='base64-encoded-fragment_1'; > BINLOG 2, 'binlog_fragment'; No, that's totally ridiculous. A statement that internally walks the list of user variables and collects values from variables named according to a specific pattern, seriously? Please, make it BINLOG CONCAT(@binlog_fragment_0, @binlog_fragment_1) that'll work with no questions asked, everybody understands what it means. The parser doesn't need to accept an arbitrary expression here, it'd be simpler and safer to hard-code the syntax as above. A couple of minor generic comments below, but I've stopped reviewing, because the code will change anyway. > to represent a big BINLOG 'base64-encoded-"total"'. > Two more statements are composed to promptly release memory > SET @binlog_fragment_0=NULL; > SET @binlog_fragment_1=NULL; > > diff --git a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc > index 9753125dd67..f0689631cfb 100644 > --- a/client/mysqlbinlog.cc > +++ b/client/mysqlbinlog.cc > @@ -56,7 +56,13 @@ Rpl_filter *binlog_filter= 0; > > #define BIN_LOG_HEADER_SIZE 4 > #define PROBE_HEADER_LEN (EVENT_LEN_OFFSET+4) > - > +/* > + 2 fragments can always represent near 1GB row-based base64-encoded event as > + two strings each of size less than max(max_allowed_packet). > + Bigger number of fragments does not safe from potential need to tune (increase) > + @@max_allowed_packet before to process the fragments. So 2 is safe and enough. > +*/ > +#define BINLOG_ROWS_EVENT_ENCODED_FRAGMENTS 2 > > #define CLIENT_CAPABILITIES (CLIENT_LONG_PASSWORD | CLIENT_LONG_FLAG | CLIENT_LOCAL_FILES) > > @@ -71,6 +77,9 @@ ulong bytes_sent = 0L, bytes_received = 0L; > ulong mysqld_net_retry_count = 10L; > ulong open_files_limit; > ulong opt_binlog_rows_event_max_size; > +#ifndef DBUG_OFF > +ulong opt_binlog_rows_event_max_encoded_size; > +#endif Try to avoid unnecessary ifdefs. Here you could've defined the variable unconditionally and suppressed the warning with __attribute__((unused)), of you could've used it unconditionally as a splitting threshold below and only hidden the command line option. > uint test_flags = 0; > static uint opt_protocol= 0; > static FILE *result_file; > @@ -813,7 +822,14 @@ write_event_header_and_base64(Log_event *ev, FILE *result_file, > > /* Write header and base64 output to cache */ > ev->print_header(head, print_event_info, FALSE); > - ev->print_base64(body, print_event_info, FALSE); > + > + /* the assert states the only current use case for the function */ > + DBUG_ASSERT(print_event_info->base64_output_mode == > + BASE64_OUTPUT_ALWAYS); 1. don't document the obvious 2. don't wrap the line if it fits in 80-char line > + > + ev->print_base64(body, print_event_info, > + print_event_info->base64_output_mode != > + BASE64_OUTPUT_DECODE_ROWS); > > /* Read data from cache and write to result file */ > if (copy_event_cache_to_file_and_reinit(head, result_file) || > @@ -1472,6 +1490,15 @@ that may lead to an endless loop.", > "This value must be a multiple of 256.", > &opt_binlog_rows_event_max_size, &opt_binlog_rows_event_max_size, 0, > GET_ULONG, REQUIRED_ARG, UINT_MAX, 256, ULONG_MAX, 0, 256, 0}, > +#ifndef DBUG_OFF > + {"binlog-row-event-max-encoded-size", 0, all debug command-line options must start with "debug-", that is, either rename it to "debug-binlog-row-event-max-encoded-size" or make it non-debug. > + "The maximum size of base64-encoded rows-event in one BINLOG pseudo-query " > + "instance. When the computed actual size exceeds the limit " > + "the BINLOG's argument string is fragmented in two.", > + &opt_binlog_rows_event_max_encoded_size, > + &opt_binlog_rows_event_max_encoded_size, 0, > + GET_ULONG, REQUIRED_ARG, UINT_MAX/4, 256, ULONG_MAX, 0, 256, 0}, > +#endif > {"verify-binlog-checksum", 'c', "Verify checksum binlog events.", > (uchar**) &opt_verify_binlog_checksum, (uchar**) &opt_verify_binlog_checksum, > 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 3

Re: [Maria-developers] [Commits] 2f4a0c5be2c: Fix accumulation of old rows in mysql.gtid_slave_pos
by Kristian Nielsen 13 Oct '18

13 Oct '18

Hi Andrei! Can you review this patch? This fixes a rather serious bug in parallel replication that was reported on maria-discuss@ here: https://lists.launchpad.net/maria-discuss/msg05253.html The user ended up with millions of rows in mysql.gtid_slave_pos, which is quite bad. I think this should go in 10.1. The bug is probably also present in 10.0, but it will be much harder to trigger since 10.0 does not have optimistic parallel replication. So leaving it unfixed in 10.0 seems ok. I plan to manually merge it to 10.2 and 10.3 as well, since the merge to 10.3 is not entirely trivial, due to interaction with @@gtid_pos_auto_engines. Github branch here: https://github.com/knielsen/server/commits/gtid_table_garbage_rows - Kristian. Kristian Nielsen <knielsen(a)knielsen-hq.org> writes: > revision-id: 2f4a0c5be2c5d5153c4253a49ba8820ab333a9a0 (mariadb-10.1.35-71-g2f4a0c5be2c) > parent(s): 1fc5a6f30c3a9c047dcf9a36b00026d98f286f6b > author: Kristian Nielsen > committer: Kristian Nielsen > timestamp: 2018-10-07 18:59:52 +0200 > message: > > Fix accumulation of old rows in mysql.gtid_slave_pos > > This would happen especially in optimistic parallel replication, where there > is a good chance that a transaction will be rolled back (due to conflicts) > after it has executed record_gtid(). If the transaction did any deletions of > old rows as part of record_gtid(), those deletions will be undone as well. > And the code did not properly ensure that the deletions would be re-tried. > > This patch makes record_gtid() remember the list of deletions done as part > of a transaction. Then in rpl_slave_state::update() when the changes have > been committed, we discard the list. However, in case of error and rollback, > in cleanup_context() we will instead put the list back into > rpl_global_gtid_slave_state so that the deletions will be re-tried later. > > Probably fixes part of the cause of MDEV-12147 as well. > > Signed-off-by: Kristian Nielsen <knielsen(a)knielsen-hq.org> > > --- > .../suite/rpl/r/rpl_parallel_optimistic.result | 6 ++ > .../suite/rpl/t/rpl_parallel_optimistic.test | 17 +++++ > sql/log_event.cc | 6 +- > sql/rpl_gtid.cc | 64 ++++++++++++------ > sql/rpl_gtid.h | 2 +- > sql/rpl_rli.cc | 79 ++++++++++++++++++++++ > sql/rpl_rli.h | 11 +++ > 7 files changed, 161 insertions(+), 24 deletions(-) > > diff --git a/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result b/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result > index 3cd4f8231bf..99bd8562ffe 100644 > --- a/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result > +++ b/mysql-test/suite/rpl/r/rpl_parallel_optimistic.result > @@ -571,4 +571,10 @@ SET GLOBAL slave_parallel_mode=@old_parallel_mode; > SET GLOBAL slave_parallel_threads=@old_parallel_threads; > include/start_slave.inc > DROP TABLE t1, t2, t3; > +include/save_master_gtid.inc > +include/sync_with_master_gtid.inc > +Check that no more than the expected last two GTIDs are in mysql.gtid_slave_pos > +select count(*) from mysql.gtid_slave_pos order by domain_id, sub_id; > +count(*) > +2 > include/rpl_end.inc > diff --git a/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test b/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test > index 9f6669279db..3867a3fdf3a 100644 > --- a/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test > +++ b/mysql-test/suite/rpl/t/rpl_parallel_optimistic.test > @@ -549,5 +549,22 @@ SET GLOBAL slave_parallel_threads=@old_parallel_threads; > > --connection server_1 > DROP TABLE t1, t2, t3; > +--source include/save_master_gtid.inc > + > +--connection server_2 > +--source include/sync_with_master_gtid.inc > +# Check for left-over rows in table mysql.gtid_slave_pos (MDEV-12147). > +# > +# There was a bug when a transaction got a conflict and was rolled back. It > +# might have also handled deletion of some old rows, and these deletions would > +# then also be rolled back. And since the deletes were never re-tried, old no > +# longer needed rows would accumulate in the table without limit. > +# > +# The earlier part of this test file have plenty of transactions being rolled > +# back. But the last DROP TABLE statement runs on its own and should never > +# conflict, thus at this point the mysql.gtid_slave_pos table should be clean. > +--echo Check that no more than the expected last two GTIDs are in mysql.gtid_slave_pos > +select count(*) from mysql.gtid_slave_pos order by domain_id, sub_id; > > +--connection server_1 > --source include/rpl_end.inc > diff --git a/sql/log_event.cc b/sql/log_event.cc > index e1912ad4620..e07b7002398 100644 > --- a/sql/log_event.cc > +++ b/sql/log_event.cc > @@ -4429,7 +4429,7 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi, > > gtid= rgi->current_gtid; > if (rpl_global_gtid_slave_state->record_gtid(thd, &gtid, sub_id, > - true, false)) > + rgi, false)) > { > int errcode= thd->get_stmt_da()->sql_errno(); > if (!is_parallel_retry_error(rgi, errcode)) > @@ -7132,7 +7132,7 @@ Gtid_list_log_event::do_apply_event(rpl_group_info *rgi) > { > if ((ret= rpl_global_gtid_slave_state->record_gtid(thd, &list[i], > sub_id_list[i], > - false, false))) > + NULL, false))) > return ret; > rpl_global_gtid_slave_state->update_state_hash(sub_id_list[i], &list[i], > NULL); > @@ -7639,7 +7639,7 @@ int Xid_log_event::do_apply_event(rpl_group_info *rgi) > rgi->gtid_pending= false; > > gtid= rgi->current_gtid; > - err= rpl_global_gtid_slave_state->record_gtid(thd, &gtid, sub_id, true, > + err= rpl_global_gtid_slave_state->record_gtid(thd, &gtid, sub_id, rgi, > false); > if (err) > { > diff --git a/sql/rpl_gtid.cc b/sql/rpl_gtid.cc > index 7b1acf17ef5..94944b5b3e5 100644 > --- a/sql/rpl_gtid.cc > +++ b/sql/rpl_gtid.cc > @@ -77,7 +77,7 @@ rpl_slave_state::record_and_update_gtid(THD *thd, rpl_group_info *rgi) > rgi->gtid_pending= false; > if (rgi->gtid_ignore_duplicate_state!=rpl_group_info::GTID_DUPLICATE_IGNORE) > { > - if (record_gtid(thd, &rgi->current_gtid, sub_id, false, false)) > + if (record_gtid(thd, &rgi->current_gtid, sub_id, NULL, false)) > DBUG_RETURN(1); > update_state_hash(sub_id, &rgi->current_gtid, rgi); > } > @@ -328,6 +328,8 @@ rpl_slave_state::update(uint32 domain_id, uint32 server_id, uint64 sub_id, > } > } > rgi->gtid_ignore_duplicate_state= rpl_group_info::GTID_DUPLICATE_NULL; > + > + rgi->pending_gtid_deletes_clear(); > } > > if (!(list_elem= (list_element *)my_malloc(sizeof(*list_elem), MYF(MY_WME)))) > @@ -377,15 +379,24 @@ int > rpl_slave_state::put_back_list(uint32 domain_id, list_element *list) > { > element *e; > + int err= 0; > + > + mysql_mutex_lock(&LOCK_slave_state); > if (!(e= (element *)my_hash_search(&hash, (const uchar *)&domain_id, 0))) > - return 1; > + { > + err= 1; > + goto end; > + } > while (list) > { > list_element *next= list->next; > e->add(list); > list= next; > } > - return 0; > + > +end: > + mysql_mutex_unlock(&LOCK_slave_state); > + return err; > } > > > @@ -468,12 +479,12 @@ gtid_check_rpl_slave_state_table(TABLE *table) > /* > Write a gtid to the replication slave state table. > > - Do it as part of the transaction, to get slave crash safety, or as a separate > - transaction if !in_transaction (eg. MyISAM or DDL). > - > gtid The global transaction id for this event group. > sub_id Value allocated within the sub_id when the event group was > read (sub_id must be consistent with commit order in master binlog). > + rgi rpl_group_info context, if we are recording the gtid transactionally > + as part of replicating a transactional event. NULL if called from > + outside of a replicated transaction. > > Note that caller must later ensure that the new gtid and sub_id is inserted > into the appropriate HASH element with rpl_slave_state.add(), so that it can > @@ -481,13 +492,13 @@ gtid_check_rpl_slave_state_table(TABLE *table) > */ > int > rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > - bool in_transaction, bool in_statement) > + rpl_group_info *rgi, bool in_statement) > { > TABLE_LIST tlist; > int err= 0; > bool table_opened= false; > TABLE *table; > - list_element *elist= 0, *next; > + list_element *elist= 0, *cur, *next; > element *elem; > ulonglong thd_saved_option= thd->variables.option_bits; > Query_tables_list lex_backup; > @@ -558,7 +569,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > thd->wsrep_ignore_table= true; > #endif > > - if (!in_transaction) > + if (!rgi) > { > DBUG_PRINT("info", ("resetting OPTION_BEGIN")); > thd->variables.option_bits&= > @@ -601,9 +612,9 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > if ((elist= elem->grab_list()) != NULL) > { > /* Delete any old stuff, but keep around the most recent one. */ > - list_element *cur= elist; > - uint64 best_sub_id= cur->sub_id; > + uint64 best_sub_id= elist->sub_id; > list_element **best_ptr_ptr= &elist; > + cur= elist; > while ((next= cur->next)) > { > if (next->sub_id > best_sub_id) > @@ -636,7 +647,8 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > table->file->print_error(err, MYF(0)); > goto end; > } > - while (elist) > + cur = elist; > + while (cur) > { > uchar key_buffer[4+8]; > > @@ -646,9 +658,9 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > /* `break' does not work inside DBUG_EXECUTE_IF */ > goto dbug_break; }); > > - next= elist->next; > + next= cur->next; > > - table->field[1]->store(elist->sub_id, true); > + table->field[1]->store(cur->sub_id, true); > /* domain_id is already set in table->record[0] from write_row() above. */ > key_copy(key_buffer, table->record[0], &table->key_info[0], 0, false); > if (table->file->ha_index_read_map(table->record[1], key_buffer, > @@ -662,8 +674,7 @@ rpl_slave_state::record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > not want to endlessly error on the same element in case of table > corruption or such. > */ > - my_free(elist); > - elist= next; > + cur= next; > if (err) > break; > } > @@ -686,18 +697,31 @@ IF_DBUG(dbug_break:, ) > */ > if (elist) > { > - mysql_mutex_lock(&LOCK_slave_state); > put_back_list(gtid->domain_id, elist); > - mysql_mutex_unlock(&LOCK_slave_state); > + elist = 0; > } > > ha_rollback_trans(thd, FALSE); > } > close_thread_tables(thd); > - if (in_transaction) > + if (rgi) > + { > thd->mdl_context.release_statement_locks(); > + /* > + Save the list of old gtid entries we deleted. If this transaction > + fails later for some reason and is rolled back, the deletion of those > + entries will be rolled back as well, and we will need to put them back > + on the to-be-deleted list so we can re-do the deletion. Otherwise > + redundant rows in mysql.gtid_slave_pos may accumulate if transactions > + are rolled back and retried after record_gtid(). > + */ > + rgi->pending_gtid_deletes_save(gtid->domain_id, elist); > + } > else > + { > thd->mdl_context.release_transactional_locks(); > + rpl_group_info::pending_gtid_deletes_free(elist); > + } > } > thd->lex->restore_backup_query_tables_list(&lex_backup); > thd->variables.option_bits= thd_saved_option; > @@ -1080,7 +1104,7 @@ rpl_slave_state::load(THD *thd, char *state_from_master, size_t len, > > if (gtid_parser_helper(&state_from_master, end, &gtid) || > !(sub_id= next_sub_id(gtid.domain_id)) || > - record_gtid(thd, &gtid, sub_id, false, in_statement) || > + record_gtid(thd, &gtid, sub_id, NULL, in_statement) || > update(gtid.domain_id, gtid.server_id, sub_id, gtid.seq_no, NULL)) > return 1; > if (state_from_master == end) > diff --git a/sql/rpl_gtid.h b/sql/rpl_gtid.h > index 79d566bddbf..7bd639b768f 100644 > --- a/sql/rpl_gtid.h > +++ b/sql/rpl_gtid.h > @@ -182,7 +182,7 @@ struct rpl_slave_state > uint64 seq_no, rpl_group_info *rgi); > int truncate_state_table(THD *thd); > int record_gtid(THD *thd, const rpl_gtid *gtid, uint64 sub_id, > - bool in_transaction, bool in_statement); > + rpl_group_info *rgi, bool in_statement); > uint64 next_sub_id(uint32 domain_id); > int iterate(int (*cb)(rpl_gtid *, void *), void *data, > rpl_gtid *extra_gtids, uint32 num_extra, > diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc > index 64a1b535307..b35130c1505 100644 > --- a/sql/rpl_rli.cc > +++ b/sql/rpl_rli.cc > @@ -1680,6 +1680,7 @@ rpl_group_info::reinit(Relay_log_info *rli) > long_find_row_note_printed= false; > did_mark_start_commit= false; > gtid_ev_flags2= 0; > + pending_gtid_delete_list= NULL; > last_master_timestamp = 0; > gtid_ignore_duplicate_state= GTID_DUPLICATE_NULL; > speculation= SPECULATE_NO; > @@ -1804,6 +1805,12 @@ void rpl_group_info::cleanup_context(THD *thd, bool error) > erroneously update the GTID position. > */ > gtid_pending= false; > + > + /* > + Rollback will have undone any deletions of old rows we might have made > + in mysql.gtid_slave_pos. Put those rows back on the list to be deleted. > + */ > + pending_gtid_deletes_put_back(); > } > m_table_map.clear_tables(); > slave_close_thread_tables(thd); > @@ -2027,6 +2034,78 @@ rpl_group_info::unmark_start_commit() > } > > > +/* > + When record_gtid() has deleted any old rows from the table > + mysql.gtid_slave_pos as part of a replicated transaction, save the list of > + rows deleted here. > + > + If later the transaction fails (eg. optimistic parallel replication), the > + deletes will be undone when the transaction is rolled back. Then we can > + put back the list of rows into the rpl_global_gtid_slave_state, so that > + we can re-do the deletes and avoid accumulating old rows in the table. > +*/ > +void > +rpl_group_info::pending_gtid_deletes_save(uint32 domain_id, > + rpl_slave_state::list_element *list) > +{ > + /* > + We should never get to a state where we try to save a new pending list of > + gtid deletes while we still have an old one. But make sure we handle it > + anyway just in case, so we avoid leaving stray entries in the > + mysql.gtid_slave_pos table. > + */ > + DBUG_ASSERT(!pending_gtid_delete_list); > + if (unlikely(pending_gtid_delete_list)) > + pending_gtid_deletes_put_back(); > + > + pending_gtid_delete_list= list; > + pending_gtid_delete_list_domain= domain_id; > +} > + > + > +/* > + Take the list recorded by pending_gtid_deletes_save() and put it back into > + rpl_global_gtid_slave_state. This is needed if deletion of the rows was > + rolled back due to transaction failure. > +*/ > +void > +rpl_group_info::pending_gtid_deletes_put_back() > +{ > + if (pending_gtid_delete_list) > + { > + rpl_global_gtid_slave_state->put_back_list(pending_gtid_delete_list_domain, > + pending_gtid_delete_list); > + pending_gtid_delete_list= NULL; > + } > +} > + > + > +/* > + Free the list recorded by pending_gtid_deletes_save(). Done when the deletes > + in the list have been permanently committed. > +*/ > +void > +rpl_group_info::pending_gtid_deletes_clear() > +{ > + pending_gtid_deletes_free(pending_gtid_delete_list); > + pending_gtid_delete_list= NULL; > +} > + > + > +void > +rpl_group_info::pending_gtid_deletes_free(rpl_slave_state::list_element *list) > +{ > + rpl_slave_state::list_element *next; > + > + while (list) > + { > + next= list->next; > + my_free(list); > + list= next; > + } > +} > + > + > rpl_sql_thread_info::rpl_sql_thread_info(Rpl_filter *filter) > : rpl_filter(filter) > { > diff --git a/sql/rpl_rli.h b/sql/rpl_rli.h > index 74d5b6fe416..b40a34a54e6 100644 > --- a/sql/rpl_rli.h > +++ b/sql/rpl_rli.h > @@ -676,6 +676,11 @@ struct rpl_group_info > /* Needs room for "Gtid D-S-N\x00". */ > char gtid_info_buf[5+10+1+10+1+20+1]; > > + /* List of not yet committed deletions in mysql.gtid_slave_pos. */ > + rpl_slave_state::list_element *pending_gtid_delete_list; > + /* Domain associated with pending_gtid_delete_list. */ > + uint32 pending_gtid_delete_list_domain; > + > /* > The timestamp, from the master, of the commit event. > Used to do delayed update of rli->last_master_timestamp, for getting > @@ -817,6 +822,12 @@ struct rpl_group_info > char *gtid_info(); > void unmark_start_commit(); > > + static void pending_gtid_deletes_free(rpl_slave_state::list_element *list); > + void pending_gtid_deletes_save(uint32 domain_id, > + rpl_slave_state::list_element *list); > + void pending_gtid_deletes_put_back(); > + void pending_gtid_deletes_clear(); > + > time_t get_row_stmt_start_timestamp() > { > return row_stmt_start_timestamp; > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

2 5

[Maria-developers] SP memory usage
by jerome brauge 12 Oct '18

12 Oct '18

Hello Alexander, As stored procedures seems to be very memory aggressive, I found two possible optimizations to avoid having three strings containing the stored procedure code in memory. Probably, same optimizations may be applied to packages. I haven't add any tests because I think everything is covered by the existing tests. What do you think about ? In a second time, I think working on SP cache. Currently it' s a simple hash table and all the hash table is flushed as soon as the threshold (stored_program_cache) is reached. It's not very good. We could : - Use a more intelligent cache (LFU ?) - Add some performance counters Regards. Ce message et les pi?ces jointes sont confidentiels et ?tablis ? l'attention exclusive de ses destinataires. Toute utilisation ou diffusion, m?me partielle, non autoris?e est interdite. Tout message ?lectronique est susceptible d'alt?ration; CEGID d?cline donc toute responsabilit? au titre de ce message. Si vous n'?tes pas le destinataire de ce message, merci de le d?truire et d'avertir l'exp?diteur. This message and any attachments are confidential and intended solely for the addressees. Any unauthorized use or disclosure, either whole or partial is prohibited. E-mails are susceptible to alteration; CEGID shall therefore not be liable for the content of this message. If you are not the intended recipient of this message, please delete it and notify the sender.

1 0

[Maria-developers] [beginner] how can I assign an existing ticket to myself?
by Takashi Sasaki 12 Oct '18

12 Oct '18

Hello, I wanted to contribute something to MariaDB, so I signed up to Jira, but how can I assign an existing ticket to myself? [programming experience]: I have been programming for over 10 years but recently using Java, Python, Node.js at work and C ++ is a little played a few years ago. [knowledge of the MariaDB source] I just started reading the code recently and I am almost an amateur. [how much you know about using MySQL/MariaDB] I use MySQL for WordPress backend as a user and I use Oracle for work, so I have knowledge of general RDBMS, but I do not have deep knowledge specific to the products. Thanks, Takashi Sasaki

3 2

Re: [Maria-developers] [Commits] 1edec241473: MDEV-17137: Syntax errors with VIEW using MEDIAN
by Sergey Petrunia 11 Oct '18

11 Oct '18

Hi Varun, On Wed, Sep 12, 2018 at 06:13:23PM +0530, Varun wrote: > revision-id: 1edec241473a122553fa9a27be995f3aef52654e (mariadb-10.3.7-171-g1edec241473) > parent(s): 5a1868b58d26b286b6ad433096e7184895953311 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2018-09-12 18:10:16 +0530 > message: > > MDEV-17137: Syntax errors with VIEW using MEDIAN > > The syntax error happened because we had not implemented a different print for > percentile functions. The syntax is a bit different when we use percentile functions > as window functions in comparision to normal window functions. > Implemented a seperate print function for percentile functions > ... > diff --git a/sql/item_windowfunc.h b/sql/item_windowfunc.h > index b3e23748246..268182a2788 100644 > --- a/sql/item_windowfunc.h > +++ b/sql/item_windowfunc.h > @@ -1311,6 +1311,7 @@ class Item_window_func : public Item_func_or_sum > bool resolve_window_name(THD *thd); > > void print(String *str, enum_query_type query_type); > + void print_percentile_functions(String *str, enum_query_type query_type); > * Please make this function private * I think "print_percentile_functions" is a bad name as it implies multiple functions are printed. In fact, only one function is printed. How about "print_for_percentile_functions"? Ok to push after the above is addressed. BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0