Re: [Maria-developers] c3ef531aaff: MDEV-10963 Fragmented BINLOG query
Hi, Andrei! Thanks! We're almost there. just a couple of minor comments re. format strings, see below. On Dec 18, andrei.elkin@pp.inet.fi wrote:
revision-id: c3ef531aaff39d3262882213157d36a545b600dd (mariadb-10.1.37-31-gc3ef531aaff) parent(s): 541500295abdba7fa619065069291ac9c1dd6e83 author: Andrei Elkin committer: Andrei Elkin timestamp: 2018-12-18 18:28:59 +0200 message:
MDEV-10963 Fragmented BINLOG query ... diff --git a/sql/log_event.cc b/sql/log_event.cc --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -10491,12 +10505,128 @@ void Rows_log_event::pack_info(Protocol *protocol) #endif
#ifdef MYSQL_CLIENT +/** + Print an event "body" cache to @c file possibly in two fragments. + Each fragement is optionally per @c do_wrap to produce an SQL statement. + + @param file a file to print to + @param body the "body" IO_CACHE of event + @param do_wrap whether to wrap base64-encoded strings with + SQL cover. + @param delimiter delimiter string + + The function signals on any error through setting @c body->error to -1. +*/ +void copy_cache_to_file_wrapped(FILE *file, + IO_CACHE *body, + bool do_wrap, + const char *delimiter) +{ + const char str_binlog[]= "\nBINLOG '\n"; + const char fmt_delim[]= "'%s\n"; + const char fmt_n_delim[]= "\n'%s\n"; + const my_off_t cache_size= my_b_tell(body); + + if (reinit_io_cache(body, READ_CACHE, 0L, FALSE, FALSE)) + { + body->error= -1; + goto end; + } + + if (!do_wrap) + { + my_b_copy_to_file(body, file, SIZE_T_MAX); + } + else if (4 + sizeof(str_binlog) + cache_size + sizeof(fmt_delim) > + opt_binlog_rows_event_max_encoded_size) + { + /* + 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. + + Split the big query when its packet size's estimation exceeds a + limit. The estimate includes the maximum packet header + contribution of non-compressed packet. + */ + const char fmt_frag[]= "%sSET @binlog_fragment_%d ='\n"; + + my_fprintf(file, fmt_frag, "\n", 0); + if (my_b_copy_to_file(body, file, cache_size/2 + 1)) + { + body->error= -1; + goto end; + } + my_fprintf(file, fmt_n_delim, delimiter);
see, you use fmt_n_delim only once. here. but you end it with \n. And it forced you to start fmt_frag with %s. please, remove \n from the end of fmt_n_delim and add it to the beginning of fmt_frag
+ + my_fprintf(file, fmt_frag, "", 1); + if (my_b_copy_to_file(body, file, SIZE_T_MAX)) + { + body->error= -1; + goto end; + } + my_fprintf(file, fmt_delim, delimiter);
and here - no \n before ' ? I've got lost in all these new lines and format constants. can you please show what the binlog dump will look like?
+ + my_fprintf(file, "BINLOG @binlog_fragment_%d, @binlog_fragment_%d%s\n", + 0, 1, delimiter);
don't pass 0 and 1 as parameters, it's not like they can ever change :)
+ } + else + { + my_fprintf(file, str_binlog); + if (my_b_copy_to_file(body, file, SIZE_T_MAX)) + { + body->error= -1; + goto end; + } + my_fprintf(file, fmt_delim, delimiter); + } + reinit_io_cache(body, WRITE_CACHE, 0, FALSE, TRUE); + +end: + return; +} + +/** + 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 @binlog_fragment_0, @binlog_fragment_1; + + where fragments are represented by a pair of indexed user + "one shot" variables. + + NOTE.
still @note
+ If any changes made don't forget to duplicate them to + Old_rows_log_event as long as it's supported. + + @param file pointer to IO_CACHE + @param print_event_info pointer to print_event_info specializing + what out of and how to print the event + @param name the name of a table that the event operates on + + The function signals on any error of cache access through setting + that cache's @c error to -1. +*/ 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);
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (1)
-
Sergei Golubchik