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

[Feature][Checkpoint] Add check script for source/sink state class serialVersionUID missing #9111

Closed
wants to merge 8 commits into from

Conversation

CosmosNi
Copy link
Contributor

@CosmosNi CosmosNi commented Apr 3, 2025

Add check script for source/sink state class serialVersionUID missing
close #9080

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@hailin0 hailin0 requested a review from Copilot April 3, 2025 05:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request adds a check script to verify that all state classes in the connectors define proper serialVersionUID values and updates several class files to replace missing or placeholder values.

  • Added or updated serialVersionUID in multiple connector classes.
  • Introduced tests to enforce proper serialVersionUID usage via a new SerialVersionUIDCheckerTest.

Reviewed Changes

Copilot reviewed 90 out of 91 changed files in this pull request and generated no comments.

Show a summary per file
File Description
connector-elasticsearch/ElasticsearchSourceSplit.java Replaces -1L with a valid serialVersionUID.
connector-easysearch/EasysearchSinkState.java Adds serialVersionUID to ensure serialization compatibility.
connector-easysearch/EasysearchCommitInfo.java Introduces serialVersionUID field.
connector-easysearch/EasysearchAggregatedCommitInfo.java Adds serialVersionUID field.
connector-easysearch/EasysearchSourceState.java Adds serialVersionUID field.
connector-doris/DorisSinkState.java Adds serialVersionUID field.
connector-doris/DorisCommitInfo.java Adds serialVersionUID field.
connector-common/SingleSplitEnumeratorState.java Adds serialVersionUID field.
connector-common/SingleSplit.java Adds serialVersionUID field.
connector-clickhouse/ClickhouseSinkState.java Adds serialVersionUID field.
connector-clickhouse/CKFileCommitInfo.java Adds serialVersionUID field.
connector-clickhouse/CKFileAggCommitInfo.java Adds serialVersionUID field.
connector-clickhouse/CKCommitInfo.java Adds serialVersionUID field.
connector-clickhouse/CKAggCommitInfo.java Adds serialVersionUID field.
connector-cdc/tidb/TiDBSourceCheckpointState.java Adds serialVersionUID field.
connector-cdc-base/SourceSplitBase.java Adds serialVersionUID field.
connector-amazondynamodb/AmazonDynamoDBSourceState.java Adds serialVersionUID field.
connector-amazondynamodb/AmazonDynamoDBSourceSplit.java Adds serialVersionUID field.
seatunnel-ci-tools/SerialVersionUIDCheckerTest.java Introduces test to check serialVersionUID usage and validate error messaging.
Files not reviewed (1)
  • seatunnel-ci-tools/pom.xml: Language not supported
Comments suppressed due to low confidence (1)

seatunnel-ci-tools/src/test/java/org/apache/seatunnel/api/SerialVersionUIDCheckerTest.java:268

  • The error message includes 'Question:' which appears to be a typo; consider replacing it with 'classes:' for clarity.
errorMessage.append("A total of ").append(missingSerialVersionUID.size()).append(" Question:\n\n");

@@ -28,6 +28,7 @@
@Setter
public class AmazonDynamoDBSourceSplit implements SourceSplit {

private static final long serialVersionUID = -5148142613656330674L;
Copy link
Member

Choose a reason for hiding this comment

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

How is it generated? Is it consistent with the automatically generated id?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Automatically generate using idea.
image

Copy link
Member

@hailin0 hailin0 Apr 3, 2025

Choose a reason for hiding this comment

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

To be compatible restore checkpoint with older versions, you should get the id automatically generated by the old version

import java.io.ObjectStreamClass;

public class SerialVersionUIDChecker {
    public static void main(String[] args) {
        long serialVersionUID = ObjectStreamClass.lookup(AmazonDynamoDBSourceSplit.class).getSerialVersionUID();
        System.out.println("serialVersionUID = " + serialVersionUID);
    }
}

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

@CosmosNi CosmosNi Apr 7, 2025

Choose a reason for hiding this comment

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

ObjectStreamClass.lookup(AmazonDynamoDBSourceSplit.class).getSerialVersionUID();
This seem doesn't to work.

Copy link
Member

Choose a reason for hiding this comment

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

How do you ensure that your changes remain compatible with old versions of task restore checkpoint?

Copy link
Contributor Author

@CosmosNi CosmosNi Apr 7, 2025

Choose a reason for hiding this comment

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

image I first removed the idea generated,and I tested several, and this seems to be consistent with the one generated by idea

Copy link
Member

Choose a reason for hiding this comment

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

It is recommended that you use all serialVersionUID generated by ObjectStreamClass

@CosmosNi CosmosNi closed this Apr 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature][Checkpoint] Add check script for source/sink state class serialVersionUID missing
2 participants