rdblue commented on a change in pull request #497: Support retaining last N 
snapshots
URL: https://github.com/apache/incubator-iceberg/pull/497#discussion_r332292685
 
 

 ##########
 File path: core/src/main/java/org/apache/iceberg/RemoveSnapshots.java
 ##########
 @@ -77,8 +79,34 @@ public ExpireSnapshots expireSnapshotId(long 
expireSnapshotId) {
 
   @Override
   public ExpireSnapshots expireOlderThan(long timestampMillis) {
-    LOG.info("Expiring snapshots older than: {} ({})", new 
Date(timestampMillis), timestampMillis);
-    this.expireOlderThan = timestampMillis;
+    updateExpireOlderThan(timestampMillis, true);
+    return this;
+  }
+
+  private boolean updateExpireOlderThan(long timestampMillis, boolean 
fromExpireOlderThan) {
+    if (expireOlderThan == null ||
+        expireOlderThan > timestampMillis ||
+        this.isExpireOlderThanSet == fromExpireOlderThan) {
+      LOG.info("Expiring snapshots older than: {} ({})", new 
Date(timestampMillis), timestampMillis);
+      this.expireOlderThan = timestampMillis;
+      this.isExpireOlderThanSet = fromExpireOlderThan;
+      return true;
+    }
+    return false;
+  }
+
+  @Override
+  public ExpireSnapshots retainLast(int numSnapshots) {
 
 Review comment:
   I think that `retainLast` should designate what to keep. If 
`expireOlderThan` is not called, then no snapshots will be expired.
   
   Also, if we choose to default both of these settings from table properties 
(e.g. age in days to expire), then the behavior would be to use the table 
settings as the default and then override with these calls. If neither the 
table expiration age setting nor `expireOlderThan` are used, then no snapshots 
would expire.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to