amirmor1 commented on code in PR #17164: URL: https://github.com/apache/iceberg/pull/17164#discussion_r3774062619
########## core/src/main/java/org/apache/iceberg/BaseMetastoreTableOperations.java: ########## @@ -55,6 +55,7 @@ public abstract class BaseMetastoreTableOperations extends BaseMetastoreOperatio private String currentMetadataLocation = null; private boolean shouldRefresh = true; private int version = -1; + private volatile Long metadataFileSizeInBytes; Review Comment: Agreed, this was the weakest part of the change. Reading shared ops state at report time meant the value wasn't bound to the snapshot being reported, and it also left a stale size in place on any commit path that doesn't write a metadata file. I've moved the capture into the producer, following the `newSnapshotId` pattern already in `SnapshotProducer#commit`: ```java AtomicReference<Long> metadataFileSizeInBytes = new AtomicReference<>(); ... taskOps.commit(base, updated.withUUID()); metadataFileSizeInBytes.set(taskOps.metadataFileSizeInBytes()); ... notifyListeners(metadataFileSizeInBytes.get()); ``` The size is now read in the same thread immediately after the commit attempt that produced it, and it's scoped to a single producer. A concurrent commit through a shared ops instance can no longer be attributed to this snapshot at report time, and stale values from earlier commits are gone. Added `eachCommitReportsTheMetadataFileSizeItWrote` to cover the per-commit scoping. This still routes the value through a field on the ops instance, which I kept because `TableOperations#commit` returns void and the write happens inside subclass `doCommit` implementations. -- 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]
