DBZ-5930: Support snapshot feature without an automatic retry #112

yoheimuta · 2022-12-13T02:39:47Z

ref. https://issues.redhat.com/browse/DBZ-5930

This PR adjusts the event state machine in gRPC responses to support the snapshot feature.

The following three cases differ from the streaming and should be taken care of.

Duplicate VGTID when the VStream Copy starts
Duplicate BEGIN when the VStream Copy starts and the table has no rows
Duplicate BEGIN when the VStream Copy starts and more than one table has no rows

…m Copy (taking a snapshot)

yoheimuta · 2022-12-13T02:50:23Z

src/main/java/io/debezium/connector/vitess/connection/VitessReplicationConnection.java

-                                        event.getVgtid().toString());
+                                if (newVgtid.getRawVgtid().getShardGtidsList().stream().findFirst().map(s -> s.getTablePKsCount() == 0).orElse(false)
+                                    && event.getVgtid().getShardGtidsList().stream().findFirst().map(s -> 0 < s.getTablePKsCount()).orElse(false)) {
+                                    LOGGER.info("Received more than one VGTID events during a copy operation and the previous one is {}. Using the latest: {}",


The actual log is:

2022-12-12 18:57:05,028 INFO || Received more than one VGTID events during a copy operation and the previous one is [{"keyspace":"test_unsharded_keyspace","shard":"0","gtid":"MySQL56/4edb14ad-79c3-11ed-a748-0242ac110002:1-947"}]. Using the latest: shard_gtids { keyspace: "test_unsharded_keyspace" shard: "0" gtid: "MySQL56/4edb14ad-79c3-11ed-a748-0242ac110002:1-947" table_p_ks { table_name: "t1" lastpk { rows { lengths: 1 values: "1" } } } }

jpechane · 2022-12-14T12:13:17Z

@yoheimuta Hi, thanks for the PR. It looks relatively easy to be done. For the sake of cross-connector consistency, could you please take a look at snapshot.mode config option used by other connectors? If possible I'd like to drive/configure snapshotting via it rather than with a special value of GTID setting.

Also could you please make sure you run the build locally as it formats the source code too.

yoheimuta · 2022-12-15T02:30:15Z

Thank you for telling me!
I'm looking into snapshot.mode and formatting.

…h the other connectors

…hot.mode

yoheimuta · 2022-12-15T09:46:22Z

src/test/java/io/debezium/connector/vitess/VitessConnectorIT.java

+    private void startConnector(Function<Configuration.Builder, Configuration.Builder> customConfig,
+                                boolean hasMultipleShards, boolean offsetStoragePerTask,
+                                int numTasks, int gen, int prevNumTasks, String tableInclude,
+                                VitessConnectorConfig.SnapshotMode snapshotMode)


[memo] When you give null as snapshotMode here, the connector uses the default snapshotMode (INITIAL).

yoheimuta · 2022-12-15T09:51:40Z

@jpechane Hi, I fixed these two points.

formating: 87a8c12
snapshot.mode: 7cfb6a0 f2c59a5 b12e5b7

Could you please review this PR?

yoheimuta · 2022-12-15T09:53:43Z

src/main/java/io/debezium/connector/vitess/VitessConnectorConfig.java

+    /**
+     * The set of predefined SnapshotMode options or aliases.
+     */
+    public static enum SnapshotMode implements EnumeratedValue {


I took a look at https://debezium.io/documentation/reference/2.0/connectors/mongodb.html#mongodb-property-snapshot-mode.

yoheimuta · 2022-12-15T09:55:10Z

src/main/java/io/debezium/connector/vitess/VitessConnectorConfig.java

@@ -210,6 +278,17 @@ public class VitessConnectorConfig extends RelationalDatabaseConnectorConfig {
                            + "Once we persist the new offsets in offset storage using new partition key "
                            + "based on current <numTasks> and <gen>, we will no longer read prev.num.tasks param");

+    public static final Field SNAPSHOT_MODE = Field.create("snapshot.mode")
+            .withDisplayName("Snapshot mode")
+            .withEnum(SnapshotMode.class, SnapshotMode.INITIAL)


All the other connectors set the default to INITIAL.

HenryCaiHaiying · 2022-12-16T05:03:18Z

src/main/java/io/debezium/connector/vitess/VitessConnectorConfig.java

@@ -296,6 +376,9 @@ public String getShard() {
    }

    public String getGtid() {
+        if (getSnapshotMode() == SnapshotMode.INITIAL) {


SnapshotMode would be Initial be default. Would this mean by default we will pull using gtid="" which is full table scan, this would be a big performance impact on vitess side if people are not careful about their configs.

Also the caller of getGtid is VitessReplicationConnection.defaultVGtid(). There are three code paths in defaultVGtid() (offsetStoragePerTask mode, shard = "", shard is not empty) config.getGtid() only covers one of the paths, can the gtid="" also works for the other two paths?

@HenryCaiHaiying I don't think this is wrong. Other connectors also execute full table scans upon connector start as they do the snapshot of existing content.

can the gtid="" also works for the other two paths?

shard = ""

This case is not supported by the current Vitess now. I reported it to the Vitess team.
The fix will be included in Vitess 16 by vitessio/vitess#11886.

So, I added a warning output and a test for now 70a89be.

offsetStoragePerTask mode

Thanks! offsetStoragePerTask mode slipped my mind.
Maybe it should be supported. I'm looking into it.

I have some concerns on the defaulting behavior. The defaulting snapshot pull will change the existing system's behavior. The current default is to pull from the tail of the BinLog, now when people upgrade to the newer version of debezium and if they are not careful, it becomes pull from the head of the BinLog with a big performance impact.

The other issue is the GTID in the config is being ignored when the snapshot mode is Initial (which is the default). Sometimes we use that GTID config parameter to have the connector to rewind to an old position in the binlog, now this function is lost if we forgot to change the snapshot mode. I think the system should honor the GTID param if it's set, only use "" if gtd is not present (flip your current fallback logic)

I kind of like the first version of the PR where the user will explicitly set the GTID param to "" to indicate he wants to pull from the beginning of the binlog. This wouldn't cause confusions or performance impact if not careful.

offsetStoragePerTask mode

Maybe it should be supported. I'm looking into it.

I added a commit 822a58d to support it.

jpechane · 2022-12-16T07:54:26Z

@yoheimuta One key question, could you please add a test that will verify what will happen after connector restart? I am a bit afraid that if the state of snapshot being completed is not recorded in offsets then it will be reexcuted again but I might be wrong.
Still it is is important to test it.

…fig doesn't specify shards

yoheimuta · 2022-12-16T09:08:08Z

@jpechane Thanks! It makes sense.
I'm adding a test that verifies what will happen after the connector restart.

And I will also add a test that will start copying without a table include filter.
I found out this case sent a different event sequence.

…after the connector finished it and restarted

…s and more than one table has no rows

yoheimuta · 2022-12-16T11:18:05Z

@jpechane Hi, I added the test: 689a2c5 f3d5551.

HenryCaiHaiying · 2022-12-17T06:31:26Z

src/main/java/io/debezium/connector/vitess/connection/VitessReplicationConnection.java

@@ -352,6 +370,9 @@ public static Vgtid defaultVgtid(VitessConnectorConfig config) {
        }
        else {
            if (config.getShard() == null || config.getShard().isEmpty()) {
+                if (config.getGtid().equals("")) {


Although vitess doesn't support empty shard and empty gtid position, there is a workaround possible in debezium connector side. We can get all the shards from vtgate and replace the empty shard param with all shards and continue (this was actually what was happening in VtGate code when you don't specify shard). VitessConnector.getVitessShards() can get you all the shards.

That sounds nice!
I was slanted toward that the connector side code was simpler and we would wait for the fix in Vitess.

But, given that this use case is recommended at the document and the current code has VitessConnector.getVitessShards(), I changed my mind that it's worth implementing the workaround now.
So, I'm getting to it.

I added two commits 5c7a7f6 3043a9c to take snapshots from all shards.

…fsetStoragePerTask is true

… the config doesn't specify shards" This reverts commit 70a89be.

…oth shard and gtid are empty

HenryCaiHaiying · 2022-12-20T05:45:52Z

src/main/java/io/debezium/connector/vitess/VitessConnector.java

@@ -150,7 +150,7 @@ public List<Map<String, String>> taskConfigs(int maxTasks, List<String> shards)
                    }
                    if (gtidStr == null) {
                        LOGGER.warn("No previous gtid found either for shard: {}, fallback to current", shard);


Can you change the order of line 152 and 153, so you can print out the gtidStr value in the LOGGER.warn?

Thanks! I overlooked it.
Added 5ea8780.

Thanks, looks good to me.

ex. 2022-12-20 14:57:53,936 WARN VitessConnectorIT||engine No previous gtid found either for shard: 0, fallback to '' [io.debezium.connector.vitess.VitessConnector]

HenryCaiHaiying · 2022-12-20T06:12:32Z

@yoheimuta The change looks good to me overall, thanks for adding support for empty shard and OffsetStoragePerTask mode. I have some concerns on changing the initial pull behavior to snapshot the whole table instead of from tail because of the performance impact , but if you guys feel this is consistent with the other connectors, you can move forward but the release note has to clearly specify this changing behavior and the potential performance impact on the database side.

jpechane · 2022-12-20T13:53:35Z

@HenryCaiHaiying According to the test the initial snapshot should not be executed after connector upgrade. In that case the current installations are safe and the new installations will have the same behaviour as the other connectors.
Yes, it will be mentioned in release notes under breaking changes section.

yoheimuta · 2022-12-21T03:42:48Z

@HenryCaiHaiying @jpechane Thank you for your careful review!
I agree with clearly specifying this breaking change behavior.

@jpechane So, should I provide a docs update PR to the main repo as the next step?

HenryCaiHaiying · 2022-12-21T04:25:41Z

@yoheimuta Thanks for the contribution. It was nice someone can push though the changes on both debezium and vitess community.

jpechane · 2022-12-21T12:12:16Z

@yoheimuta Applied, thanks! yes, please prepare the docs PR too.

yoheimuta · 2022-12-22T09:29:30Z

See corresponding doc PR: debezium/debezium#4158

Enable support for enums during vstream copy phase. There are two reasons that the connector does not handle `enum` for PSDB branches. 1. The upstream debezium-connector-vitess simply does not support `enum` during the VStream copy phase. It tries to cast the row value to an integer, but the value is a string. It seems support for `enum` landed in 2021 debezium#20, and support for snapshots (VStream Copy) landed in 2022 debezium#112, without taking the former into account. This is easily fixed by finding finding the index of the string value in the list of values obtained from `column_type` during the schema discovery phase at the beginning of the VStream. 2. However, this isn't working on some PSDB branches which don't have the fix vitessio/vitess#13045 for this bug vitessio/vitess#12981. Fixable by backporting the bugfix or upgrading those branches. Signed-off-by: Max Englander <[email protected]>

DBZ-5930: Adjust the event state machine to gRPC responses for VStrea…

8d65d48

…m Copy (taking a snapshot)

yoheimuta commented Dec 13, 2022

View reviewed changes

yoheimuta added 4 commits December 15, 2022 11:39

DBZ-5930: Run mvn formatter:format

87a8c12

DBZ-5930: Support snapshot.mode to allow users to configure snapshotting

7cfb6a0

DBZ-5930: Set the default snapshot.mode to INITIAL to get in line wit…

f2c59a5

…h the other connectors

DBZ-5930: Adjust the description of gtid to the introduction of snaps…

b12e5b7

…hot.mode

yoheimuta commented Dec 15, 2022

View reviewed changes

HenryCaiHaiying reviewed Dec 16, 2022

View reviewed changes

DBZ-5930: Output a warning when trying to take a snapshot and the con…

70a89be

…fig doesn't specify shards

yoheimuta added 2 commits December 16, 2022 19:55

DBZ-5930: Add a test that verifies a snapshotting is not re-executed …

689a2c5

…after the connector finished it and restarted

DBZ-5930: Adjust the gRPC response events when the VStream Copy start…

498895d

…s and more than one table has no rows

HenryCaiHaiying reviewed Dec 17, 2022

View reviewed changes

yoheimuta added 4 commits December 19, 2022 15:18

DBZ-5930: Set gtid to an empty string when the mode is initial and of…

822a58d

…fsetStoragePerTask is true

DBZ-5930: Fix a test that fails only when running all the tests together

f3d5551

Revert "DBZ-5930: Output a warning when trying to take a snapshot and…

5c7a7f6

… the config doesn't specify shards" This reverts commit 70a89be.

DBZ-5930: Implement a workaround to get all shards from vtgate when b…

3043a9c

…oth shard and gtid are empty

HenryCaiHaiying reviewed Dec 20, 2022

View reviewed changes

DBZ-5930: Print out the fallbacked gtidStr in the LOGGER.warn

5ea8780

ex. 2022-12-20 14:57:53,936 WARN VitessConnectorIT||engine No previous gtid found either for shard: 0, fallback to '' [io.debezium.connector.vitess.VitessConnector]

jpechane merged commit 61903a1 into debezium:main Dec 21, 2022

yoheimuta deleted the support-snapshot branch December 22, 2022 02:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DBZ-5930: Support snapshot feature without an automatic retry #112

DBZ-5930: Support snapshot feature without an automatic retry #112

yoheimuta commented Dec 13, 2022 •

edited

Loading

yoheimuta Dec 13, 2022

jpechane commented Dec 14, 2022 •

edited

Loading

yoheimuta commented Dec 15, 2022

yoheimuta Dec 15, 2022

yoheimuta commented Dec 15, 2022

yoheimuta Dec 15, 2022

yoheimuta Dec 15, 2022

HenryCaiHaiying Dec 16, 2022

HenryCaiHaiying Dec 16, 2022

jpechane Dec 16, 2022

yoheimuta Dec 16, 2022

yoheimuta Dec 16, 2022

HenryCaiHaiying Dec 17, 2022

yoheimuta Dec 19, 2022

jpechane commented Dec 16, 2022

yoheimuta commented Dec 16, 2022 •

edited

Loading

yoheimuta commented Dec 16, 2022 •

edited

Loading

HenryCaiHaiying Dec 17, 2022

yoheimuta Dec 19, 2022

yoheimuta Dec 20, 2022

HenryCaiHaiying Dec 20, 2022

yoheimuta Dec 20, 2022

HenryCaiHaiying Dec 20, 2022

HenryCaiHaiying commented Dec 20, 2022

jpechane commented Dec 20, 2022

yoheimuta commented Dec 21, 2022

HenryCaiHaiying commented Dec 21, 2022

jpechane commented Dec 21, 2022

yoheimuta commented Dec 22, 2022

DBZ-5930: Support snapshot feature without an automatic retry #112

DBZ-5930: Support snapshot feature without an automatic retry #112

Conversation

yoheimuta commented Dec 13, 2022 • edited Loading

Choose a reason for hiding this comment

jpechane commented Dec 14, 2022 • edited Loading

yoheimuta commented Dec 15, 2022

Choose a reason for hiding this comment

yoheimuta commented Dec 15, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jpechane commented Dec 16, 2022

yoheimuta commented Dec 16, 2022 • edited Loading

yoheimuta commented Dec 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HenryCaiHaiying commented Dec 20, 2022

jpechane commented Dec 20, 2022

yoheimuta commented Dec 21, 2022

HenryCaiHaiying commented Dec 21, 2022

jpechane commented Dec 21, 2022

yoheimuta commented Dec 22, 2022

yoheimuta commented Dec 13, 2022 •

edited

Loading

jpechane commented Dec 14, 2022 •

edited

Loading

yoheimuta commented Dec 16, 2022 •

edited

Loading

yoheimuta commented Dec 16, 2022 •

edited

Loading