Hello Sachin, Andrei,
Andrei, thanks for your analysis! Sachin, can you please try to implement according to Andrei's proporal? Thanks! On 10/18/2018 03:57 PM, andrei.el...@pp.inet.fi wrote: > Alexander, Sachin, hello. > >> Hi Andrei, >> >> We need your help on: >> >> MDEV-10362 mysqlbinlog verbose output cannot be decoded >> >> >> The problem is that "mysqlbinlog -vvv" writes >> comments right inside the BINLOG statement, >> between base64 chunks: >> >> >> BINLOG ' >> kxiEVxMBAAAALQAAAMUBAAAAABMAAAAAAAEABHRlc3QAAnQxAAMIDwMCIAAG >> ... >> AAAAAAAAAQAAAPo/AAAAAAAAAAAAAAD6QAAAAAAAAAABAAAA+kAAAAAAAAAAAAAAAA== >> ### UPDATE `test`.`t1` >> ### WHERE >> ### @1=1 /* LONGINT meta=0 nullable=0 is_null=0 */ >> ### @2=NULL /* LONGINT meta=32 nullable=1 is_null=1 */ >> ### @3=1 /* INT meta=0 nullable=1 is_null=0 */ >> AAAAAAD63QUAAAAAAAABAAAA+t0FAAAAAAAAAAAAAPreBQAAAAAAAAEAAAD63gUAAAAAAAAAAAAA >> '/*!*/; > > Right, the comments get embedded into base64 stream for no reason. > >> >> >> So when later we try to load this output to "mysql", it fails, >> because when mysql_client_binlog_statement() calls base64_decode(), >> the latter fails as it does not expect any comments. >> >> Sachin proposes to add a new flag to base64_decode(), to make >> skip comments when this flags is on. >> >> I'm not sure that we should do this. >> >> Would it be possible to fix mysqlbinlog instead, to print: >> - a BINLOG statement with only base64 data, without any comments inside >> - followed by comments, fully outside of the BINLOG statement. >> ? > > A sort of what you both are suggesting, let's add up one more cache, > pass it to print_base64(body, vcom, print_event_info,...) [ actually, > do we really need the cache args as long as they represent > `print_event_info' members? - to check out closely] > > > 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; > + IO_CACHE ... vcom= ... // vcom after Verbose COMments > 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); > print_header(head, print_event_info, !last_stmt_event); > my_b_printf(head, "\t%s: table id %lu%s\n", > name, m_table_id, > last_stmt_event ? " flags: STMT_END_F" : ""); > - print_base64(body, print_event_info, do_print_encoded); > + print_base64(body, vcom, print_event_info, do_print_encoded); > > so inside > > void Log_event::print_base64(IO_CACHE* file, > + IO_CACHE* vcom, > PRINT_EVENT_INFO* print_event_info, > bool do_print_encoded) > > ... > > if (print_event_info->verbose) > { > ... > if (ev) > { > - ev->print_verbose(file, print_event_info); > + ev->print_verbose(vcom, print_event_info); > > > Finally, empty up the vcom cache in a correct order. > Should it be before BINLOG '...' I think, right? > > > if (get_flags(STMT_END_F)) > { > ... > copy_event_cache_to_file_and_reinit(head, file); > + copy_event_cache_to_file_and_reinit(vcom, file); > copy_event_cache_to_file_and_reinit(body, file); > > Sachin, regardless whether you like this elaboration, > you got to check out bb-10.1-MDEV-10963 or the latest > patch in the commit list to find some refactoring in this area. > There's a chance it would be pushed before the current task's one. > > Cheers, > > Andrei > > >> >> Or even, should not we split such BINLOG statement with multiple >> base64 chunks into individual BINLOG statements? >> >> >> Thanks. >> >> >> On 10/16/2018 04:03 PM, Sachin Setiya wrote: >>> Hi Bar! >>> On Tue, Oct 16, 2018 at 7:33 AM Alexander Barkov <b...@mariadb.com> wrote: >>>> >>>> Hi Sachin, >>>> >>>> >>>> On 10/16/2018 03:49 AM, Sachin Setiya wrote: >>>>> Hi Bar! >>>>> On Tue, Sep 11, 2018 at 1:47 PM Alexander Barkov <b...@mariadb.com> wrote: >>>>>> >>>>>> Hi Sachin, >>>>>> >>>>>> >>>>>> On 09/10/2018 03:47 PM, Sachin Setia wrote: >>>>>>> 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. >>>>>> >>>>>> Note, base64_decode() is used in at least two places: >>>>>> - for the binary log >>>>>> - for the SQL function FROM_BASE64() >>>>>> >>>>>> >>>>>> I don't like that your patch changes the behavior of the SQL function >>>>>> FROM_BASE64(). It should not recognize any '#' inside the base64 data >>>>>> as comments. >>>>> Right , So I have creates a flag MY_BASE64_DECODE_ALLOW_COMMENTS it is >>>>> kind of similar to as MY_BASE64_DECODE_ALLOW_MULTIPLE_CHUNKS flag is used >>>>> So no impact on FROM_BASE64 >>>>> >>>>>> >>>>>> So perhaps this should be fixed in some other place, not in >>>>>> base64_decode(). >>>>>> >>>>>> >>>>>> I checked the output of these commands: >>>>>> >>>>>> ./bin/mysqlbinlog ./data/retsina-bin.000003 >>>>>> ./bin/mysqlbinlog -vvv ./data/retsina-bin.000003 >>>>>> >>>>>> It looks suspicious for me. >>>>>> >>>>>> Can you please remind why mysqlbinlog prints multiple base64 chunks >>>>>> inside the same BINLOG statement? >>>>> It is beacuse if we have long row we can have more then one >>>>> ROWS_LOG_EVENT after TABLE_MAP_EVENT >>>> >>>> Do you know where in the code the server decides to end >>>> the current chunk and start a new one? >>> Here In binlog_prepare_pending_rows_event >>> if (!pending || >>> pending->server_id != serv_id || >>> pending->get_table_id() != table->s->table_map_id || >>> pending->get_general_type_code() != general_type_code || >>> pending->get_data_size() + needed > opt_binlog_rows_event_max_size || >>> ^^^ this line >>> pending->get_width() != colcnt || >>> !bitmap_cmp(pending->get_cols(), cols)) >>> { >>> >>>> >>>> Is there some limit (in bytes, or in number or records)? >>>> >>>>>> >>>>>> >>>>>> From my understanding, it is "mysqlbinlog -vvv" who should be fixed >>>>>> to print good base64 data inside the string literal that follows the >>>>>> BINLOG keyword. >>>>>> >>>>>> At least the additional comments printed by -vvv should be outside of >>>>>> the BINLOG statement (presumably before), not inside. >>>>>> >>>>> Idk , this can be lot of work , plus it will slow mysqlbinlog also , >>>>> And I think with new solution FROM_BASE64 behavior is not changed >>>>> I have added test for FROM_BASE64 >>>>> #No change in BASE64_FROM behaviour >>>>> SELECT FROM_BASE64('TWFy >>>>> #Comment 344 >>>>> # >>>>> aWE=') AS 'Output'; >>>>> this will generate Maria in earlier patch which is wrong , with new >>>>> patch it gives NULL >>>> >>>> If we really go this way, I suggest to cover all these scenarios too: >>>> >>>> SELECT FROM_BASE64('TWFyaWE=# comment1'); >>>> SELECT FROM_BASE64('TWFyaWE=\n# comment1'); >>>> SELECT FROM_BASE64('TWFyaWE=\n# comment1\nTWFyaWE='); >>>> SELECT FROM_BASE64('TWFyaWE=\n# comment1\nTWFyaWE=\n# comment2\n'); >>>> >>>> They all should give NULL. >>> They all are giving null >>>> >>>>>> Thanks. >>>>>> >>>>>> _______________________________________________ >>>>>> Mailing list: https://launchpad.net/~maria-developers >>>>>> Post to : maria-developers@lists.launchpad.net >>>>>> Unsubscribe : https://launchpad.net/~maria-developers >>>>>> More help : https://help.launchpad.net/ListHelp >>>>> >>>>> >>>>> >>> >>> >>> _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp