Hi, Andrei! On Jun 03, Andrei Elkin wrote:

...

...

- binlog_hton->commit= binlog_commit; + binlog_hton->commit= binlog_commit_dummy; binlog_hton->rollback= binlog_rollback; binlog_hton->drop_table= [](handlerton *, const char*) { return -1; };

Oh did you actually need it? Or it's for completeness only?

I am confused a bit, 6c52931680a4 (Sergei Golubchik 2020-06-22 1703) binlog_hton->drop_table= [](handlerton *, const char*) { return -1; };

And I never reviewed the patch.

Grrr. Sorry! When reviewing I diffed this patch with a previous version that I reviewed (a useful speedup technique at the late phase, when commits only change slightly) and this drop_table line was in the new patch and not in the old. Of course, it was because the new patch was in 10.6 and the old in 10.2, there were quite a few changes in the diff because of that. I tried to ignore them, but somehow missed this one. Note, there's no + before the drop_table line, it's not yours, it's indeed something that already was in 10.6, I should've noticed, my mistake :( Why I added it in 6c52931680a4? It was for DROP TABLE FORCE, that simply does plugin_foreach() and tries to hton->drop_table() in every hton available. In binlog_hton too. The goal was to solve cases like a table in InnoDB data dictionary but with no frm, or Aria table where MAD file exists, but no MAI - cases when DROP TABLE doesn't see the table, but CREATE TABLE still fails as some remnants of a table still exist.

...

...

@@ -9975,6 +9977,154 @@ int TC_LOG::using_heuristic_recover() + else + { + char buf[21]; + my_stat(file_name, &s, MYF(0)); + my_off_t new_size= s.st_size;

do you need it? new_size == pos, isn't it? You yourself use the word "size" to describe pos in sql_print_error just few lines above :)

Sure. Let turn it into an

assert( pos == new_size )

[x]

and this is, what I think, you'd call "too much checks" :) I'd personally would've dropped my_stat and new_size, especially as I also suggest to remove them from the sql_print_information below.

...

...

+ + longlong10_to_str(ptr_gtid->seq_no, buf, 10); + sql_print_information("Successfully truncated binlog file:%s " + "to pos:%llu to remove transactions starting from " + "GTID %u-%u-%s; " + "former file size shrunk from %llu to %llu.",

This isn't quite English. Better just "previous (or old) file size: %llu", that is not "former" and no need to specify the new size twice.

Also, you say that you "truncated binlog file" here, but on failure you fail to "trim the binlog file". I suggest to use consistent wording, either always "trim" or always "truncate" (and always with or without "the") - both on success and on failure. Personally, I favor "truncate the binlog file"

some two styles of two writers of these blocks mixed up :-). (Partly relates to the size in above).

[x]

Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org