Hi Kristian,
The table map version was used in the early implementation to identify
when it was necessary to write out a table map, but I think that the
current logic does not require the table map at all.
I've been reluctant to remove it since I have not had time to sit down
and make sure that it is not needed any more.
Best wishes,
Mats Kindahl
Kristian Nielsen wrote:
> Hi,
>
> I am looking at the function binlog_end_trans() in sql/log.cc in MySQL 5.1.44,
> and the similar functions binlog_flush_trx_cache() and
> binlog_truncate_trx_cache() in latest mysql-trunk.
>
> This has the following code:
>
> /*
> We need to step the table map version after writing the
> transaction cache to disk.
> */
> mysql_bin_log.update_table_map_version();
>
> ...
>
> /*
> We need to step the table map version on a rollback to ensure
> that a new table map event is generated instead of the one that
> was written to the thrown-away transaction cache.
> */
> mysql_bin_log.update_table_map_version();
>
>
> My question is if this is really necessary? It seems that since the table map
> version is also updated in MYSQL_BIN_LOG::flush_and_set_pending_rows_event(),
> it should not be necessary to update it again here?
>
> And if it _is_ necessary, it looks like a bug, as the update in
> binlog_end_trans() is done without any locking:
>
> void update_table_map_version() { ++m_table_map_version; }
>
> (the update in flush_and_set_pending_rows_event() is protected by LOCK_log in
> MySQL 5.1.44).
>
> So it seems to me that those update_table_map_version() calls are either
> unnecessary or incorrect. Comments?
>
> In mysql_trunk, all updates of table_map_version are now unprotected by any
> lock. I do not see how this can be correct? Espcially since it is a 64-bit
> number, so we could even imagine (on 32-bit machines) seeing the counter go
> from 0xfffffffe to 0x0 in case of a race.
>
> The locking around table_map_version update was removed in this commit:
>
> revid:alfranio.correia@stripped
>
> This removes the locking and the following comment without any explanation
> that I could find:
>
> /*
> If we are writing to the log file directly, we could avoid
> locking the log. This does not work since we need to step the
> m_table_map_version below, and that change has to be protected
> by the LOCK_log mutex.
> */
>
> So can anyone help me with understanding:
>
> 1. If/why the table_map_version in binlog_end_trans is necessary?
>
> 2. If/why it is safe to update table_map_version without locks in 5.5?
>
> - Kristian.
>
--
Mats Kindahl
Senior Software Engineer
Database Technology Group
Sun Microsystems
