Hi! <cut>
To ensure that this is not wrongly used, please add #ifdef MYSQL_CLIENT
here:
class Log_event:
#ifdef MYSQL_CLIENT + enum enum_binlog_checksum_alg read_checksum_alg; #endif
Yes, agree, that is the intention - the read_checksum_alg is only present in the class in the client, not in the server (and we want to avoid using memory for it in the server).
This code is already in the #else branch of #ifdef MYSQL_SERVER:
#ifdef MYSQL_SERVER ... #else /* The checksum algorithm used (if any) when the event was read. */ enum enum_binlog_checksum_alg read_checksum_alg;
Sorry, I missed the above #ifdef when reading the patch (for this case I didn't look at the source. Please ignore the wrong suggestion. <cut>
in Format_description_log_event(const uchar
We have:
else { checksum_alg= BINLOG_CHECKSUM_ALG_UNDEF; used_checksum_alg= BINLOG_CHECKSUM_ALG_OFF; }
The used_checksum_alg is not needed as you set it at function start.
Sorry, I don't understand? At the start we set it to "undef":
+ used_checksum_alg= BINLOG_CHECKSUM_ALG_UNDEF;
This value will remain if there's an error return; else the else branch will change it to "off".
Did you miss that it's set differently at the start of the function and in this else { } branch? Or did I miss something?
Another mistake from my side. I missed that you had removed checksum_alg= BINLOG_CHECKSUM_ALG_UNDEF; <cut>
I'll fix it, remove the function set_checksum_alg() and just do:
org_checksum_length= writer->checksum_length; writer->checksum_length= BINLOG_CHECKSUM_LEN; ... writer->checksum_length= org_checksum_length;
Great! <cut>
Format_description_log_event::write(Log_event_writer *writer)
uint8 checksum_byte= (uint8) (used_checksum_alg != BINLOG_CHECKSUM_ALG_UNDEF ? used_checksum_alg : BINLOG_CHECKSUM_ALG_OFF);
Is it possible the alg can be UNDEF? You have several asserts in the code that this should not happen.
Yeah, honestly, I'm not sure. It's one of the annoying things in the code that I did not get to fix, that we have both the UNDEF and the OFF state for the checksum algorithm.
Format_description code is not performance critical (only once per binlog file), so I suggest I leave the extra check for UNDEF as defensive code, but add an assertion that it is not UNDEF and see if it passes the testsuite. Does that sound ok?
Yes, adding an assert would be good. If we never hit it for some time, we can then later remove the test for BINLOG_CHECKSUM_ALG_UNDEF. <cut>
slave.cc
@@ -1870,10 +1873,8 @@ static int get_master_version_and_clock(MYSQL* mysql, Master_info* mi) until it has received a new FD_m. */ mi->rli.relay_log.description_event_for_queue->checksum_alg= mi->rli.relay_log.relay_log_checksum_alg;
Not sure why this is removed. ... is replaced by this code just above which serves the same purpose:
- Format_description_log_event(4, mysql->server_version); + Format_description_log_event(4, mysql->server_version, + mi->rli.relay_log.relay_log_checksum_alg);
So I think adding an assert
DBUG_ASSERT(mi->rli.relay_log.description_event_for_queue->checksum_alg == mi->rli.relay_log.relay_log_checksum_alg);
is somewhat redundant, but let me know if you still think it will be useful and I'll add it.
Not needed, thanks for the explanation. Regards, Monty