You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "krisnaru (via GitHub)" <gi...@apache.org> on 2025/10/16 21:26:58 UTC
[PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
krisnaru opened a new pull request, #14354:
URL: https://github.com/apache/iceberg/pull/14354
### Description:
We need to add comprehensive logging and metrics instrumentation to the Iceberg expire snapshots functionality to help diagnose and prevent production issues where the expire snapshot job deletes more files than expected.
### Background:
We encountered a production issue where an expire snapshot job deleted significantly more files than expected, making the table unreadable and causing queries to fail with java.io.FileNotFoundException: File does not exist.
### Requirements:
Log the list of files being deleted during snapshot expiration
Log the reason for each data file why it was marked for deletion
Provide sufficient instrumentation to trace the root cause of unexpected file deletions
### Metrics:
Track the number of files being deleted
Monitor snapshot expiration operations
Alert on anomalies in deletion patterns
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#issuecomment-3578169251
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #14354: Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action
URL: https://github.com/apache/iceberg/pull/14354
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "laserninja (via GitHub)" <gi...@apache.org>.
laserninja commented on code in PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#discussion_r2441579309
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/ExpireSnapshotsSparkAction.java:
##########
@@ -282,4 +309,113 @@ private ExpireSnapshots.Result deleteFiles(Iterator<FileInfo> files) {
.deletedStatisticsFilesCount(summary.statisticsFilesCount())
.build();
}
+
+ /**
+ * Logs which snapshots will be expired and the reasons for expiration. This helps debug issues
+ * where more files than expected were deleted.
+ */
+ private void logSnapshotExpirationReasons(TableMetadata metadata) {
+ LOG.info("=== Snapshot Expiration Analysis ===");
+
+ // Determine snapshots that will be kept for retain_last calculation
+ Set<Long> snapshotsToKeep = Sets.newHashSet();
+ if (retainLastValue != null) {
+ List<Snapshot> sortedSnapshots =
+ metadata.snapshots().stream()
+ .sorted((s1, s2) -> Long.compare(s2.timestampMillis(), s1.timestampMillis()))
+ .collect(Collectors.toList());
+
+ sortedSnapshots.stream()
+ .limit(retainLastValue)
+ .map(Snapshot::snapshotId)
+ .forEach(snapshotsToKeep::add);
+
+ LOG.info("Snapshots to keep (retain_last={}): {}", retainLastValue, snapshotsToKeep);
+ }
+
+ // Analyze each snapshot and log expiration reason
+ for (Snapshot snapshot : metadata.snapshots()) {
+ long snapshotId = snapshot.snapshotId();
+ boolean willExpire = false;
+ String reason = "";
+
+ // Check explicit snapshot IDs
+ if (expiredSnapshotIds.contains(snapshotId)) {
+ willExpire = true;
+ reason = "explicitly marked for expiration";
+ }
+ // Check time-based criteria
+ else if (expireOlderThanValue != null && snapshot.timestampMillis() < expireOlderThanValue) {
+ willExpire = true;
+ reason = "older than threshold (" + new java.util.Date(expireOlderThanValue) + ")";
+ }
+ // Check count-based criteria
+ else if (retainLastValue != null && !snapshotsToKeep.contains(snapshotId)) {
+ willExpire = true;
+ reason = "beyond retain_last limit (" + retainLastValue + ")";
+ }
+
+ if (willExpire) {
+ LOG.info(
+ "WILL EXPIRE snapshot {} created at {} - reason: {}",
+ snapshotId,
+ new java.util.Date(snapshot.timestampMillis()),
+ reason);
+ }
+ }
+ }
+
+ /**
+ * Logs detailed information about files that will be deleted, organized by snapshot. This
+ * provides the file-to-snapshot mapping requested for debugging.
+ */
+ private void logSnapshotDeletionSummary(
+ TableMetadata originalMetadata, Set<Long> deletedSnapshotIds) {
+ LOG.info("=== File Deletion Analysis by Snapshot ===");
+
+ for (Long snapshotId : deletedSnapshotIds) {
+ Set<Long> singleSnapshotSet = Sets.newHashSet(snapshotId);
+ Dataset<FileInfo> snapshotFiles = fileDS(originalMetadata, singleSnapshotSet);
+
+ // Use memory-efficient streaming to avoid OOM with large snapshots
+ long totalFiles = snapshotFiles.count();
Review Comment:
Calling .count() on each snapshot might trigger a full dataset scan, right? It can get expensive. Is it possible to use approximate counts or add a timeout/limit mechanism?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "laserninja (via GitHub)" <gi...@apache.org>.
laserninja commented on code in PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#discussion_r2441581725
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/ExpireSnapshotsSparkAction.java:
##########
@@ -282,4 +309,113 @@ private ExpireSnapshots.Result deleteFiles(Iterator<FileInfo> files) {
.deletedStatisticsFilesCount(summary.statisticsFilesCount())
.build();
}
+
+ /**
+ * Logs which snapshots will be expired and the reasons for expiration. This helps debug issues
+ * where more files than expected were deleted.
+ */
+ private void logSnapshotExpirationReasons(TableMetadata metadata) {
+ LOG.info("=== Snapshot Expiration Analysis ===");
+
+ // Determine snapshots that will be kept for retain_last calculation
+ Set<Long> snapshotsToKeep = Sets.newHashSet();
+ if (retainLastValue != null) {
+ List<Snapshot> sortedSnapshots =
+ metadata.snapshots().stream()
Review Comment:
What happens when metadata.snapshot() returns null? Will it fail?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #14354: Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action
URL: https://github.com/apache/iceberg/pull/14354
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "laserninja (via GitHub)" <gi...@apache.org>.
laserninja commented on code in PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#discussion_r2441580868
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/ExpireSnapshotsSparkAction.java:
##########
@@ -67,6 +67,9 @@ public class ExpireSnapshotsSparkAction extends BaseSparkAction<ExpireSnapshotsS
public static final String STREAM_RESULTS = "stream-results";
public static final boolean STREAM_RESULTS_DEFAULT = false;
+ public static final String LOG_EXPIRE_FILES = "log-expire-files";
+ public static final boolean LOG_EXPIRE_FILES_DEFAULT = false;
Review Comment:
Nit: Should this be an enum `LOG_EXPIRE_FILES_DEFAULT`? Where can we have different levels of log details?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#issuecomment-4375642668
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#issuecomment-4426287662
This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#issuecomment-3544432270
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] Spark 4.0: Optional switch to log expire data files during ExpireSnapshots action [iceberg]
Posted by "laserninja (via GitHub)" <gi...@apache.org>.
laserninja commented on code in PR #14354:
URL: https://github.com/apache/iceberg/pull/14354#discussion_r2441577749
##########
spark/v4.0/spark/src/main/java/org/apache/iceberg/spark/actions/ExpireSnapshotsSparkAction.java:
##########
@@ -67,6 +67,9 @@ public class ExpireSnapshotsSparkAction extends BaseSparkAction<ExpireSnapshotsS
public static final String STREAM_RESULTS = "stream-results";
public static final boolean STREAM_RESULTS_DEFAULT = false;
+ public static final String LOG_EXPIRE_FILES = "log-expire-files";
+ public static final boolean LOG_EXPIRE_FILES_DEFAULT = false;
+ private static final int LOG_BATCH_SIZE = 10000;
Review Comment:
Were you able to test this at scale `LOG_BATCH_SIZE = 10000;`? Usually The batch size is less, right? Should we make this configurable?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org