irodriguezclaveria commented on PR #6426: URL: https://github.com/apache/paimon/pull/6426#issuecomment-4661465017
Hi @JingsongLi , Thanks for the detailed write-up. Before making any changes, I'd like to check a few things with you, because I think these may already be prevented by existing invariants and I want to make sure I'm not missing something. 1. On adding refreshed buckets to totalBucketSet / totalBucketArray: Every bucket id the refresh can surface was already loaded by loadIndex (same partition, same bucketFilter), and nothing in this class ever removes from totalBucketSet or totalBucketArray. So when we hit putIfAbsent, the id is already in both. Is there a path you have in mind where an id could appear in the refresh without ever having been in totalBucketSet? 2. On loading hash mappings for refreshed buckets: Hashes on disk under one of this assigner's buckets were either ingested by loadIndex (which does read full hash content) or written by this assigner during its current lifetime (and inserted into hash2Bucket at assignment time). Under the ownership model the bucketFilter enforces, I don't see a third case. Do you have a concrete scenario in mind where a hash could exist on disk in one of our buckets without already being in hash2Bucket? 3. Does the combination of "off by default + 24h default interval + per-partition single-flight" already meet the bar you had in mind for "conservative", or were you thinking of additional safeguards (e.g. a global cap on concurrent refreshes across partitions, or moving off the shared FileOperationThreadPool)? -- 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]
