wgtmac commented on code in PR #648:
URL: https://github.com/apache/iceberg-cpp/pull/648#discussion_r3264523096
##########
src/iceberg/update/expire_snapshots.cc:
##########
@@ -613,11 +917,23 @@ Status ExpireSnapshots::Finalize(Result<const
TableMetadata*> commit_result) {
apply_result_.reset();
// File cleanup is best-effort: log and continue on individual file deletion
failures
- ReachableFileCleanup strategy(ctx_->table->io(), delete_func_);
- return strategy.CleanFiles(metadata_before_expiration,
metadata_after_expiration,
- expired_ids, cleanup_level_);
+ // Pick incremental cleanup when the expiration is a simple linear-ancestry
walk:
+ // no explicit snapshot IDs, no removed snapshots outside main ancestry, and
no
+ // retained snapshots outside main ancestry. Mirrors Java RemoveSnapshots's
+ // dispatch in cleanExpiredSnapshots().
+ bool can_use_incremental = !specified_snapshot_id_ &&
+
!HasRemovedNonMainAncestors(metadata_before_expiration,
+
metadata_after_expiration) &&
+ !HasNonMainSnapshots(metadata_after_expiration);
+
+ std::unique_ptr<FileCleanupStrategy> strategy;
Review Comment:
This does not need heap allocation or a virtual dispatch object. Please
instantiate the selected strategy in each branch and return `CleanFiles(...)`
directly.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
