Re: [Maria-developers] 4af7a583802: Refactor MYSQL_BIN_LOG: extract Event_log ancestor

The changes to this commit are secured in 15fe904d3 https://github.com/MariaDB/server/commit/15fe904d3fd4583f1966c8a7ab6ff36111f... fixup On Tue, 23 Nov 2021 at 22:54, Nikita Malyavin wrote:

Hello Serg!

TLDR the commit is verbalized, junks are removed, `using` is preferred except for tiny getters.

On Fri, 12 Nov 2021 at 19:20, Sergei Golubchik wrote:

...

Hi, Nikita!

On Nov 12, Nikita Malyavin wrote:

...

revision-id: 4af7a583802 (mariadb-10.5.2-474-g4af7a583802) parent(s): 2da3dc5d422 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2021-01-08 20:59:09 +1000 message:

Refactor MYSQL_BIN_LOG: extract Event_log ancestor

Would be good to have some explanation here. I understand that you'll use this refactoring in following commits, but conceptually, what Event_log is?

I mean, like

Event_log is a log where Log_event_xxx events are written, but not _something else that MYSQL_BIN_LOG is and Event_log isn't_

or

Event_log is a log where Log_event_xxx events are written. While MYSQL_BIN_LOG is Event_log that also does ....

Right. Adding as follows:

Event_log is supposed to be a basic logging class that can write events in a single file.

MYSQL_BIN_LOG in comparison will have: * rotation support * index files * purging * gtid, xid and other transactional information handling. * is dedicated for a general-purpose binlog

...

...

diff --git a/sql/log.cc b/sql/log.cc index 827a34c2883..6f4e3191588 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -3701,64 +3765,25 @@ bool MYSQL_BIN_LOG::open(const char *log_name, write_file_name_to_index_file= 1; }

a bit strange that MYSQL_BIN_LOG::open doesn't use Event_log::open

Yes, I think MYSQL_BIN_LOG is left too specialized for some particular needs. I did not want to touch all that tech debt and left as is.

Besides, there's nothing weird in not calling the superclass's method.

I mean, in future commits [spoiler alert!] it will be used anyway:)

...

...

diff --git a/sql/log.h b/sql/log.h index 73cd66d5a4a..74c409e1ac7 100644 --- a/sql/log.h +++ b/sql/log.h @@ -327,6 +327,7 @@ class MYSQL_LOG bool strip_ext, char *buff); virtual int generate_new_name(char *new_name, const char *log_name, ulong next_log_number); + inline mysql_mutex_t* get_log_lock() { return &LOCK_log; } protected: /* LOCK_log is inited by init_pthread_objects() */ mysql_mutex_t LOCK_log; @@ -376,6 +377,64 @@ struct Rows_event_factory } };

+class Event_log: public MYSQL_LOG +{ +public: +#if !defined(MYSQL_CLIENT) + Rows_log_event* + prepare_pending_rows_event(THD *thd, TABLE* table, + binlog_cache_data *cache_data, + uint32 serv_id, size_t needed, + bool is_transactional, + Rows_event_factory event_factory);

forgot to delete the same from MYSQL_BIN_LOG

...

+#endif + + bool flush_and_sync(bool *synced);

same

wow, i wonder how did you notice.

...

...

...

+ void cleanup() + { + if (inited) + mysql_mutex_destroy(&LOCK_binlog_end_pos); + + MYSQL_LOG::cleanup(); + } + void init_pthread_objects() + { + MYSQL_LOG::init_pthread_objects(); + + mysql_mutex_init(m_key_LOCK_binlog_end_pos, &LOCK_binlog_end_pos, + MY_MUTEX_INIT_SLOW); + } + + bool open( + const char *log_name, + enum_log_type log_type, + const char *new_name, ulong next_file_number, + enum cache_type io_cache_type_arg); + IO_CACHE *get_log_file() { return &log_file; } + + int write_description_event(enum_binlog_checksum_alg checksum_alg, + bool encrypt, bool dont_set_created); + + bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file); +}; + /* Tell the io thread if we can delay the master info sync. */ #define SEMI_SYNC_SLAVE_DELAY_SYNC 1 /* Tell the io thread if the current event needs a ack. */ @@ -451,7 +510,7 @@ class binlog_cache_data; struct rpl_gtid; struct wait_for_commit;

-class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG +class MYSQL_BIN_LOG: public TC_LOG, private Event_log { /** The instrumentation key to use for @ LOCK_index. */ PSI_mutex_key m_key_LOCK_index; @@ -849,15 +901,13 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG

- bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file); + bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file) + { return Event_log::write_event(ev, data, file); }

Perhaps

using Event_log::write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file);

?

Indeed:)

...

...

bool write_event(Log_event *ev) { return write_event(ev, 0, &log_file); }

bool write_event_buffer(uchar* buf,uint len); @@ -918,7 +968,6 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG uint next_file_id(); inline char* get_index_fname() { return index_file_name;} inline char* get_log_fname() { return log_file_name; } - inline char* get_name() { return name; } inline mysql_mutex_t* get_log_lock() { return &LOCK_log; }

should remove that too, I reckon

Well, these are so tiny getters so I'm not sure, whether is it more comfortable to jump back and forth to check what's there in Event_log, or just admit the cost of duplication for a better read comfort. I'd leave it as it is now.

...

...

inline mysql_cond_t* get_bin_log_cond() { return &COND_bin_log_updated; } inline IO_CACHE* get_log_file() { return &log_file; }

-- Yours truly, Nikita Malyavin

-- Yours truly, Nikita Malyavin