[GitHub] [iceberg] rdblue commented on pull request #4088: Core: Set current snapshot with currentTimeMillis

[GitBox <gi...@apache.org>]

rdblue commented on pull request #4088:
URL: https://github.com/apache/iceberg/pull/4088#issuecomment-1036860261


   @wypoon are you sure that's the cause? If the timestamp is set to `null` then the timestamp used for the log is the timestamp [from the snapshot itself](https://github.com/apache/iceberg/blob/apache-iceberg-0.13.0/core/src/main/java/org/apache/iceberg/TableMetadata.java#L1181). That corresponds to the [original behavior](https://github.com/apache/iceberg/blob/7fcc71da65a47ca3c9f6eb6e862a238389b8bdc5/core/src/main/java/org/apache/iceberg/TableMetadata.java#L618-L622) and is correct. When the current snapshot is set to a newly created snapshot, we set the log timestamp to the snapshot timestamp.
   
   I think the problem is that the call to roll back is using `setCurrentSnapshot(Snapshot)` rather than `setCurrentSnapshot(long)`. The latter assumes that the snapshot is existing and sets the timestamp to `System.currentTimeMillis()`. I think the right fix here is to make sure that `SnapshotManager` ends up calling the right variant of `setCurrentSnapshot`.


-- 
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