irodriguezclaveria commented on PR #6426: URL: https://github.com/apache/paimon/pull/6426#issuecomment-4827137097
Hi @JingsongLi, Thanks for the detailed review. I have now fixed the two points you raised. 1. Refresh now updates buckets that are already present, not only absent ones Just to clarify the intent: the goal of the refresh has always been to refresh all the buckets, not only to add new (absent) ones. When I switched to put I accidentally lost that behavior, so present buckets with a stale count were no longer being refreshed. To preserve the uncommitted in-memory increments you mentioned, I track per bucket the disk row count seen at the last reconciliation (diskRowCountAtLastRefresh). The session delta is inMemory - lastDiskCount. On refresh I set the new value to diskNow + sessionDelta using an atomic compute, so: - if compaction lowered the disk count, the bucket regains capacity, and - the uncommitted in-memory increments are preserved. I also kept the conservative invariant: refresh only reconciles buckets already in totalBucketSet, so a bucket created after load is not imported. 2. Tests - Fixed the regression test so the partition is really loaded before the on-disk change. Before, the lazy loadIndex ran on the first assign (after the compaction commit), so the bucket was loaded synchronously instead of being discovered by the refresh. - Added a test where a bucket already present is reconciled down after compaction (this one fails with putIfAbsent and passes with the fix). - Added a test that a bucket created after load is not imported by the refresh. Thanks again! -- 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]
