PDGGK opened a new pull request, #9180:
URL: https://github.com/apache/paimon/pull/9180
### Purpose
Closes #9179.
Once `dynamic-bucket.max-buckets` is reached, `SimpleHashBucketAssigner` is
meant to spread further rows over the existing buckets via
`ListUtils.pickRandomly(bucketList)`. It cannot — **`bucketList` only ever
holds one element**, so the pick is a constant and every overflow row lands in
the first bucket.
`bucketList.add()` sits inside the `computeIfAbsent` mapping function. A
bucket that `loadNewBucket()` has just switched to gets its `bucketInformation`
entry created two lines later by the `compute()` in the *same* `assign()` call,
so the next call finds the key present, the mapping function never runs, and
that bucket is never registered. Only the constructor's bucket is.
With a cap of 4 and a target of 100 rows, 1000 rows produce **100 / 100 /
100 / 700**.
**Scope, precisely:** the upper bound is *not* violated — `loadNewBucket()`
guards `i <= maxBucketsNum - 1` independently. This is write skew, not a bound
violation.
**Why this is a regression rather than a design choice:** the sibling
`PartitionIndex` registers at the creation site (`totalBucketSet.add(i);
totalBucketArray.add(i)`) and behaves correctly. Both classes were given this
random-pick logic by the same commit — `cb25653f1` *"[core] Adjust
'dynamic-bucket.max-buckets' random pick logical"* — and this one lost the
registration. The fix moves the `add` into the `compute()` so it fires exactly
once per bucket, mirroring the sibling.
### Tests
Two cases added to `SimpleHashBucketAssignerTest`.
`testOverflowIsSpreadAcrossAllBuckets` asserts that **more than one bucket
ends up past its target**, which is the crispest statement of what "spread"
means. It fails on master:
```
Tests run: 11, Failures: 1
SimpleHashBucketAssignerTest.testOverflowIsSpreadAcrossAllBuckets:54
```
`testUnboundedAssignmentIsUnchanged` is the control: with `max-buckets`
unset the random-pick branch is unreachable, so the assignment sequence must be
byte-for-byte what it was. It passes **both before and after**, which is what
shows the first test is not trivially red and that the change is inert by
default.
The assertion is deterministic rather than statistical. With the bug,
`pickRandomly` over a one-element list has no randomness, so bucket 0 receives
exactly 700 with probability 1. With the fix, the test could only fail if the
RNG chose the same bucket for all 600 overflow rows — 4⁻⁶⁰⁰.
**A note on why this survived:** `testAssignWithUpperBound` asserts `isIn(0,
2)` on the overflow rows, and "always 0" satisfies that. I deliberately did not
reuse that shape.
11/11 green with the fix. Related suites pass:
`DynamicBucketIndexMaintainerTest`, `PrimaryKeySortedIndexMaintenanceTest`,
`IndexFileExpireTableTest`, `DeletionVectorsIndexFileTest`,
`BtreeGlobalIndexTableTest`. `spotless:check` and `checkstyle:check` exit 0.
--
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]
