JingsongLi commented on code in PR #2942:
URL: https://github.com/apache/incubator-paimon/pull/2942#discussion_r1512862166
##########
paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java:
##########
@@ -215,10 +204,15 @@ public int expireUntil(long earliestId, long
endExclusiveId) {
}
Snapshot snapshot = snapshotManager.snapshot(id);
- snapshotDeletion.cleanUnusedManifests(snapshot, skippingSet);
+ snapshotDeletion.cleanUnusedManifests(snapshot, skippingSet,
false);
- // delete snapshot last
-
snapshotManager.fileIO().deleteQuietly(snapshotManager.snapshotPath(id));
+ // move snapshot metadata to changelog
+ try {
+ snapshotManager.moveToChangelog(id);
+ snapshotManager.commitLongLivedChangelogLatestHint(id);
Review Comment:
Maybe we should not just move snapshot file.
We could introduce a new JSON file for changelog.
##########
paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java:
##########
@@ -215,10 +204,15 @@ public int expireUntil(long earliestId, long
endExclusiveId) {
}
Snapshot snapshot = snapshotManager.snapshot(id);
- snapshotDeletion.cleanUnusedManifests(snapshot, skippingSet);
+ snapshotDeletion.cleanUnusedManifests(snapshot, skippingSet,
false);
- // delete snapshot last
-
snapshotManager.fileIO().deleteQuietly(snapshotManager.snapshotPath(id));
+ // move snapshot metadata to changelog
+ try {
+ snapshotManager.moveToChangelog(id);
Review Comment:
Please note that it is best not to affect the original behavior. For
example, here we should determine whether there is a separate configuration for
the changelog lifecycle. If it is not configured or is the same as the snapshot
lifecycle, we should avoid creating files in the changelog folder.
##########
paimon-core/src/main/java/org/apache/paimon/table/ExpireSnapshotsImpl.java:
##########
@@ -186,17 +186,6 @@ public int expireUntil(long earliestId, long
endExclusiveId) {
snapshotDeletion.cleanUnusedDataFiles(snapshot, skipper);
}
- // delete changelog files
- for (long id = beginInclusiveId; id < endExclusiveId; id++) {
- if (LOG.isDebugEnabled()) {
- LOG.debug("Ready to delete changelog files from snapshot #" +
id);
- }
- Snapshot snapshot = snapshotManager.snapshot(id);
- if (snapshot.changelogManifestList() != null) {
Review Comment:
If possible, we should try to ensure that the delta file of the Append
snapshot can be stream read when there is no changelog file present.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
