leaves12138 commented on code in PR #8450:
URL: https://github.com/apache/paimon/pull/8450#discussion_r3519924767
##########
paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java:
##########
@@ -1400,6 +1406,25 @@ public boolean rollbackToAsLatest(Snapshot
targetSnapshot) {
return success;
}
+ private void createRollbackToAsLatestTagIfNeeded(Snapshot targetSnapshot) {
+ for (Snapshot taggedSnapshot : tagManager.tags().keySet()) {
+ if (taggedSnapshot.id() == targetSnapshot.id()) {
Review Comment:
This early return can still leave the rollback snapshot unprotected if the
existing tag is expiring. For example, if the target snapshot already has a tag
with (or an auto tag that can later be removed), rollback-to-as-latest will
not create the non-expiring tag. After snapshot expiration, that existing tag
protects the old files initially, but when /auto tag cleanup deletes the tag,
may clean files from the tagged snapshot that are still referenced by the new
rollback snapshot. I reproduced this locally by creating a tag on the target,
performing rollback-to-as-latest, expiring snapshots , then running ; reading
the rolled-back latest snapshot fails because the restored file is deleted.
Could we create a dedicated non-expiring rollback tag unless such a rollback
tag already exists, instead of skipping for any tag on the target snapshot?
##########
paimon-core/src/main/java/org/apache/paimon/operation/FileStoreCommitImpl.java:
##########
@@ -1400,6 +1406,25 @@ public boolean rollbackToAsLatest(Snapshot
targetSnapshot) {
return success;
}
+ private void createRollbackToAsLatestTagIfNeeded(Snapshot targetSnapshot) {
+ for (Snapshot taggedSnapshot : tagManager.tags().keySet()) {
+ if (taggedSnapshot.id() == targetSnapshot.id()) {
Review Comment:
This early return can still leave the rollback snapshot unprotected if the
existing tag is expiring. For example, if the target snapshot already has a tag
with `timeRetained` (or an auto tag that can later be removed),
rollback-to-as-latest will not create the non-expiring
`rollback-to-as-latest-*` tag. After snapshot expiration, that existing tag
protects the old files initially, but when `TagTimeExpire`/auto tag cleanup
deletes the tag, `TagManager.deleteTag` may clean files from the tagged
snapshot that are still referenced by the new rollback snapshot.
I reproduced this locally by creating a `Duration.ZERO` tag on the target,
performing rollback-to-as-latest, expiring snapshots `[1, 3)`, then running
`TagTimeExpire`; reading the rolled-back latest snapshot fails because the
restored file is deleted. Could we create a dedicated non-expiring rollback tag
unless such a rollback tag already exists, instead of skipping for any tag on
the target snapshot?
--
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]
