Bug Report: Do not use timestamps for schema_migrations and vreplication_log

[corbantek]

Overview of the Issue

At HubSpot we have a feature that allows us to sync data across different deployments of Vitess and is implemented by using MySQL binlogs.

When this was used in conjunction with Vitess migrations we were hitting the default timestamp bug whenever we run a new vitess migration, even though nothing we had created had default timestamps. We looked through the binlogs and sure enough, the vreplication tables have default timestamps...

After much debugging, we found that this happens whenever a CREATE TABLE/ALTER TABLE is run with a TIMESTAMP field (no matter how this was configured). The solution here is to create the column as a DATETIME field, which is virtually the same as TIMESTAMP for this use case.

Now when a vitess migration is started/stopped/completed, the CREATE TABLE/ALTER table statements are still run, but they're using datetime instead of timestamp, so the explicit_defaults_for_timestamp line does not make it into the binlogs.

Reproduction Steps

N/A

Binary Version

vitessio/main

Operating System and Environment details

MYSQL_VERSION=8.0.33

`uname -sr` 

Linux 6.1.109-hs83.el9.x86_64

Log Fragments

[shlomi-noach]

Hi! Putting some bullet numbers here so we can discuss each point separately.

1. If I'm reading this correctly, the proposal here is that Vitess should never use the TIMESTAMP data type? (thereby also in vdiff, vdiff_log, vdiff_table?)

2. From the bug report it looks like the problem is limited to using replication filters, is that correct?

3. Generally speaking, I don't see DATETIME as a good replacement to TIMESTAMP because of its lack of timezone awareness. Again generally speaking, I think in most cases TIMESTAMP is the more correct type to use.

4. I appreciate there's a MySQL bug related to timestamps. However, this also seems to be not only not in vitess, but also does not affect vitess. It only affects an external tool that uses Vitess. Is this correct?

[corbantek]

Hi! Putting some bullet numbers here so we can discuss each point separately.

4. I appreciate there's a MySQL bug related to timestamps. However, this also seems to be not only not in vitess, but also does not affect vitess. It only affects an external tool that uses Vitess. Is this correct?

Yes this is an external feature (that exists in or Vitess fork) that allows us to replicate tables to other instances of Vitess using binglogs.

1. If I'm reading this correctly, the proposal here is that Vitess should never use the TIMESTAMP data type? (thereby also in vdiff, vdiff_log, vdiff_table?)

Correct to be more complete we could update these tables, however our Cross-Vitess Replication doesn't support multi-sharded keyspaces at the moment, so these tables were not updated.

2. From the bug report it looks like the problem is limited to using replication filters, is that correct?

Correct, due to the mysqlbinlog bug, the line for explicit_defaults_for_timestamp still gets into the binlogs, which causes all the timestamp fields to get unwanted defaults

3. Generally speaking, I don't see DATETIME as a good replacement to TIMESTAMP because of its lack of timezone awareness. Again generally speaking, I think in most cases TIMESTAMP is the more correct type to use.

For our use, DATETIME is analogous with TIMESTAMP as we set everything to UTC, but I understand if someone is using timezones in a different way it would be an issue. The upside is DATETIME goes beyond 2038 :)

Our goal is upstream all of our fixes at HubSpot, but if this minor change causes more choas that good, I'm okay with keeping this forked. Maybe a compromise is have some sort of dual-mode initializer that defaults to timestamp but can also use "datetime" if a flag is supplied...