Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PLAT-6952 Test all connector options #23

Merged
merged 3 commits into from
Jan 31, 2024
Merged

PLAT-6952 Test all connector options #23

merged 3 commits into from
Jan 31, 2024

Conversation

oyeliseiev-ua
Copy link
Contributor

@oyeliseiev-ua oyeliseiev-ua commented Jan 26, 2024

Excluded:
snapshot.include.collection.list - excluded(single table currently used)
signal.data.collection excluded (pause/stop incremental snapshot is not supported)
signal.poll.interval.ms excluded
signal.enabled.channels excluded
message.key.columns - excluded(internald used)
snapshot.select.statement.overrides - excluded(observe used)
snapshot.scan.all.columns.force excluded(single table currently used)
snapshot.tables.order.by.row.count excluded(single table currently used)

Unable to alter schema during observe, but left these options as theoretically possible:
inconsistent.schema.handling.mode
event.processing.failure.handling.mode

General options(non functional unit tests coverage):
max.batch.size
max.queue.size
poll.interval.ms
max.queue.size.in.bytes
retriable.restart.connector.wait.ms
errors.max.retries
connect.timeout.ms
topic.cache.size
sourceinfo.struct.maker

@oyeliseiev-ua oyeliseiev-ua force-pushed the PLAT-6952 branch 4 times, most recently from 368c209 to 23cf6cb Compare January 26, 2024 16:01
@AdalbertMemSQL
Copy link
Collaborator

AdalbertMemSQL commented Jan 29, 2024

Looks like the inconsistent.schema.handling.mode option is never used.
I think we can exclude it for now.

@AdalbertMemSQL
Copy link
Collaborator

I found several more options that are not tested here

  • converters
  • post.processors

@oyeliseiev-ua
Copy link
Contributor Author

I found several more options that are not tested here

  • converters
  • post.processors

didn't include them because we don't have any converters or post-processors to test

@AdalbertMemSQL
Copy link
Collaborator

I found several more options that are not tested here

  • converters
  • post.processors

didn't include them because we don't have any converters or post-processors to test

Hmm…
Can the customer set a custom converter/post-processor?
If no then is there a sense of keeping this options?
BTW, what other connectors are doing with them?
Let’s just be consistent here

@oyeliseiev-ua
Copy link
Contributor Author

I found several more options that are not tested here

  • converters
  • post.processors

didn't include them because we don't have any converters or post-processors to test

Hmm… Can the customer set a custom converter/post-processor? If no then is there a sense of keeping this options? BTW, what other connectors are doing with them? Let’s just be consistent here

Converter should implement CustomConverter interface and could be used. We don't have pre-defined list of converters but customer could write own custom connector and configure it like described here https://debezium.io/documentation/reference/stable/development/converters.html#configuring-and-using-converters. Some other clients has pre-defined list of converters like TinyIntOneToBooleanConverter in Mysql.
As for post processor, there is only one implementation ReselectColumnsPostProcessor that is not applied to us. But I think for this one also could be implemented own and used.

@@ -0,0 +1,598 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you use the Eclipse XML profile?
It will allow me to use the same settings in VScode
https://stackoverflow.com/a/76326075

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, could you check if works for you?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it works.
Can you also add this to the .vscode/settings.json

{
    "java.configuration.updateBuildConfiguration": "interactive",
    "files.watcherExclude": {
        "**/target": true
    },
    "java.format.enabled": true,
    "java.format.settings.url": "./codestyle/eclipse-java-google-style.xml",
    "[java]": {
        "editor.defaultFormatter": "redhat.java"
    }
}

Comment on lines +4 to +6
import static io.debezium.config.CommonConnectorConfig.DATABASE_CONFIG_PREFIX;
import static io.debezium.config.CommonConnectorConfig.DRIVER_CONFIG_PREFIX;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this file is just reformatted.
Can we perform formatting in another PR?
It will make review process easier :)

}
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: add empty line at the end of file

assertConnectorIsRunning();

try {
conn.execute("INSERT INTO `song` VALUES ('test1', 'test1')");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something that we need to consider for the future.
Tests that are using the same tables may conflict. Need to find a way to clear info. Probably - delete everything row by row and perform a snapshot after the test.

Comment on lines 95 to 96
assertThat(mBeanServer.getAttribute(objectName, "TotalNumberOfEventsSeen")).isEqualTo(
4L);//todo fix should be 2 (create custom metrics SingleStoreDBConnectorTask?)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you file a task for this?

Comment on lines 93 to 94
assertThat(mBeanServer.getAttribute(objectName, "TotalNumberOfUpdateEventsSeen")).isEqualTo(0L);
assertThat(mBeanServer.getAttribute(objectName, "TotalNumberOfDeleteEventsSeen")).isEqualTo(0L);
Copy link
Collaborator

@AdalbertMemSQL AdalbertMemSQL Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about create and read events?

Comment on lines +73 to +75
protected void waitForStreamingToStart() throws InterruptedException {
waitForStreamingRunning("singlestoredb", "singlestore_topic");
}
Copy link
Collaborator

@AdalbertMemSQL AdalbertMemSQL Jan 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!
We can reuse it in other tests

@AdalbertMemSQL AdalbertMemSQL self-requested a review January 31, 2024 07:00
@oyeliseiev-ua oyeliseiev-ua merged commit 6a01e97 into main Jan 31, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants