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

support clone latest snapshot #3287

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wwj6591812
Copy link
Contributor

Purpose

Linked issue: close #xxx

Tests

API and Format

Documentation

@wwj6591812 wwj6591812 force-pushed the support_clone_latest_snapshot_0429 branch from b67e04e to 034753f Compare May 13, 2024 12:46
Copy link
Contributor

@tsreaper tsreaper left a comment

Choose a reason for hiding this comment

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

Your IT cases should also include copying changelog, index files, etc. Everything you copy must be tested.

You'll also need a random test with very fast expiration speed to make sure your code really works when the source table is modified at the same time.

Comment on lines +67 to +68
+ "--target_database <target_database_name> "
+ "--target_table <target_table_name> "
Copy link
Contributor

Choose a reason for hiding this comment

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

target_database and target_table are also optional.

Comment on lines +74 to +76
if (!sourceCatalogConfig.isEmpty()) {
this.sourceCatalogConfig = sourceCatalogConfig;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!sourceCatalogConfig.isEmpty()) {
this.sourceCatalogConfig = sourceCatalogConfig;
}
this.sourceCatalogConfig = sourceCatalogConfig;

Comment on lines +81 to +83
if (!targetCatalogConfig.isEmpty()) {
this.targetCatalogConfig = targetCatalogConfig;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (!targetCatalogConfig.isEmpty()) {
this.targetCatalogConfig = targetCatalogConfig;
}
this.targetCatalogConfig = targetCatalogConfig;


/**
* Pick the tables to be cloned based on the user input parameters. The record type of the build
* DataStream is Tuple2. The left element is the identifier of source table and the right element is
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* DataStream is Tuple2. The left element is the identifier of source table and the right element is
* DataStream is {@link Tuple2}. The left element is the identifier of source table and the right element is

Comment on lines +78 to +80
} catch (Exception e) {
// ignore
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't igonre exceptions. Throw it out.


import org.apache.paimon.table.sink.ChannelComputer;

/** SnapshotHintChannelComputer. */
Copy link
Contributor

Choose a reason for hiding this comment

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

Write meaningful comments please.

Comment on lines +41 to +42
* When the files copy finished of a table, then create snapshot hint, it means that this table can
* be used now.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
* When the files copy finished of a table, then create snapshot hint, it means that this table can
* be used now.
* Creates snapshot hint files after copying a table.

LOG.info(
"Skipping target file {} because it already exists and has the same size.",
targetPath);
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider copying the whole database. There are two tables in this database. All files of table A has been copied, so no record about table A will be sent to SnapshotHintOperator. Snapshot hint files of table A will not be created.

+ " dt STRING,"
+ " PRIMARY KEY (k, dt, hh) NOT ENFORCED"
+ ") PARTITIONED BY (dt, hh) WITH ("
+ " 'write-only' = 'true',"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why write only?

prepareTable(
Arrays.asList("dt", "hh"),
Arrays.asList("dt", "hh", "k"),
Collections.singletonMap(CoreOptions.WRITE_ONLY.key(), "true"));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why write only?

Run the following command to submit a clone job for the table's latest Snapshot.

```bash
CALL sys.clone('source_warehouse', 'source_database', 'source_table', '', 'target_warehouse', 'target_database', 'target_table', '', '')
Copy link
Contributor

Choose a reason for hiding this comment

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

Just support Flink 1.19. Using named argurment.

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.

None yet

3 participants