Add last updated timestamp and snapshotId for partition table #7581
Conversation
|
CC @puchengy @ajantha-bhat if you guys are interested |
| "Partition last updated timestamp"), | ||
| Types.NestedField.required( | ||
| 10, | ||
| "last_updated_snapshot_id", |
There was a problem hiding this comment.
a) Is it a good idea to keep the snapshot id? Because regularly running expire_snapshots can clean up the snapshots and we may not be able to map what operation these files were created from, even with the snapshot id.
b) There was also an ask for "latest sequence number" associated with that partition from the community users during partition stats discussion.
Do you think modified time is enough and no need for the sequence number?
There was a problem hiding this comment.
Hm, what do we think to have both? (and null if its expired, as per discussion here #7581 (comment)). If we have snapshot_id, I feel its more useful than the last_update_time, but agree we don't always have it.
I think sequence number will be good too, but do you mean fileSequenceNumber or dataSequenceNumber? Maybe worth another pr if there's more discussion there?
There was a problem hiding this comment.
I am just worried that most of the snapshots will be expired and we end up not using that field much.
The main purpose of storing the snapshot id is for finding what operation has last updated this partition id? In that case, we can store the operation type itself directly maybe.
I think sequence number will be good too, but do you mean fileSequenceNumber or dataSequenceNumber? Maybe worth another pr if there's more discussion there?
I guess it is fileSequenceNumber.
Yeah, we can have a separate discussion. I think data_file_size_in_bytes per partition can also be one more good candidate for storing here.
There was a problem hiding this comment.
a) Is it a good idea to keep the snapshot id? Because regularly running expire_snapshots can clean up the snapshots and we may not be able to map what operation these files were created from, even with the snapshot id.
b) There was also an ask for "latest sequence number" associated with that partition from the community users during partition stats discussion.
Do you think modified time is enough and no need for the sequence number?
My initial thought process is like the last updated timestamp is helpful by itself but if there's doubt around the timestamp, it's better to provide a reference to allow for further investigation. Here we derived last updated timestamp from snapshot, so providing snapshotId enable a way to look up further information about snapshot (if it's a rewrite data operation or is it an append from late arrival data).
With respect to the periodic snapshot expiration, I think partition can have null snapshot based on referenced snapshotId if it was already expired, but it seems only applicable to your data outlive your snapshot. i.e if you run data compaction along side your snapshot expiration, or if you also periodically delete your partition (like if it's daily partitioned and your dataset have a retention period) together with your snapshot expiration, it seem to be fine.
As for file sequence number, I think it might be helpful but by itself it seem to be hard to use compare to timestamp and snapshotId.
There was a problem hiding this comment.
@RussellSpitzer @aokolnychyi @flyrain any thoughts here, what would make more sense on Partition table?
There was a problem hiding this comment.
After some thoughts, I think both will be null if the snapshot is expired. But I think we are no worse off than the alternative, which is to join the entries + snapshots table for the user to find the last update time. This also will be null if snapshot is expired.
Perhaps we can have some more persistent storage of snapshot metadata , even after expire?
I think for this case (lastUpdateTime, lastSnapshotId), it is ok to proceed, typically snapshots live several days at least, and I would imagine the user of the tool is interested in the last updated partition, and can run it before the snapshot is expired.
dramaticlly
force-pushed
the
lastUpdatedSnapshot
branch
from
a1a22a3
to
8ea95c6
Compare
…stamp with respect to rewrite manifests and expire snapshot
core/src/test/java/org/apache/iceberg/MetadataTableScanTestBase.java
Outdated
Show resolved
Hide resolved
spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @dramaticlly sorry i had one more comment, just raelized
|
Looks great, will merge tomorrow if tests pass and to wait any other comments. |
spark/v3.1/spark/src/test/java/org/apache/iceberg/spark/source/TestIcebergSourceTablesBase.java
Outdated
Show resolved
Hide resolved
|
Merged, thanks @dramaticlly for the great work ! |
|
This is great! Thank you so much for adding this stat -- it is making our life so much easier. When can we expect it to be included in a release? I noticed this wasn't in 1.3.1 |
|
Yes, It will be in the upcoming 1.4.0 release. |
close #7560
What
add 2 column to partition metadata table
last_updated_atwith timestamp with timezone type (similar tocommitted_atin snapshot metadata table)last_updated_snapshot_idwith long typeHow
In Partition metadata table, we moved from iterating over ContentFiles from ManifestReader to ManifestEntry instead, since entry contain both content file and snapshotId, which are valuable to provide last updated timestamp and snapshot information. We group by file for each partition boundary and get the highest snapshot commit timestamp so that we know when exactly is the partition last modified.
Why
This can be potentially useful for iceberg table ingested by streaming application such as flink, to understand which iceberg partition is "sealed" based on timestamp. However, the write such as data compaction can also move the last update timestamp forward without actual data change, which snapshot id here can be useful for further analysis
@szehon-ho if you can help review your idea :)