Search before asking
Paimon version
master (a70912b)
Compute Engine
Flink, dynamic-bucket primary-key table. Reproduced as a unit test, so engine-independent.
Minimal reproduce step
PartitionIndex index =
new PartitionIndex(new Int2ShortHashMap(), new HashMap<>(), Long.MAX_VALUE);
int hash = 12345;
int first = index.assign(hash, b -> b >= 32768, /* maxBucketsNum */ 40000);
int second = index.assign(hash, b -> b >= 32768, /* maxBucketsNum */ 40000);
first = 32768
second = -32768
What doesn't meet your expectations?
The same key must keep the same bucket. It gets two different ones, the second of them negative.
Anything else?
PartitionIndex.assign bounds new bucket ids with
int globalMaxBucketId = (maxBucketsNum == -1 ? Short.MAX_VALUE : maxBucketsNum) - 1; // :95
so when dynamic-bucket.max-buckets is set explicitly the allocation loop can reach ids far above Short.MAX_VALUE. It then records the mapping narrowed:
hash2Bucket.put(hash, (short) i); // :103 hash2Bucket is Int2ShortHashMap
return i; // :104 not narrowed
The first record with that key is written to bucket 32768; every later record with the same key takes the memo at :75-76, which returns the short, and goes to bucket −32768. Buckets 2 and 4 narrow the same way (:88, :118).
dynamic-bucket.max-buckets is an int option whose description says it "should either be equal to -1 (unlimited), or it must be greater than 0" (CoreOptions:1694-1699). There is no upper bound in the description and no validation anywhere — git grep DYNAMIC_BUCKET_MAX_BUCKETS over main sources returns only the definition and the getter — so a value above 32767 is accepted as configured.
Note 32767 itself is fine; the first affected id is 32768. My first attempt at a reproduction used Short.MAX_VALUE as the bound and passed, which is worth mentioning in case anyone else probes this.
I did not send a patch because the three fixes I can see all trade something, and the choice looks like yours:
- Widen the memo. There is no
Int2IntHashMap in the tree, so this means a new class, and it doubles the per-key memory of a structure that is deliberately short — that cost lands on every dynamic-bucket table, including the vast majority that never exceed 32767 buckets.
- Reject the configuration in
SchemaValidation when dynamic-bucket.max-buckets > 32768. No memory cost, and it breaks no working configuration, since anything above that is already producing duplicate keys. But a job currently running with such a value would start failing to open the table rather than quietly misbehaving.
- Clamp
globalMaxBucketId to Short.MAX_VALUE and log. Keeps such jobs running and stops the corruption, at the cost of silently giving fewer buckets than asked for.
Happy to send whichever you prefer — I have the reproduction as a PartitionIndexTest ready to go with any of them.
Are you willing to submit a PR?
Search before asking
Paimon version
master (
a70912b)Compute Engine
Flink, dynamic-bucket primary-key table. Reproduced as a unit test, so engine-independent.
Minimal reproduce step
What doesn't meet your expectations?
The same key must keep the same bucket. It gets two different ones, the second of them negative.
Anything else?
PartitionIndex.assignbounds new bucket ids withso when
dynamic-bucket.max-bucketsis set explicitly the allocation loop can reach ids far aboveShort.MAX_VALUE. It then records the mapping narrowed:The first record with that key is written to bucket 32768; every later record with the same key takes the memo at
:75-76, which returns theshort, and goes to bucket −32768. Buckets 2 and 4 narrow the same way (:88,:118).dynamic-bucket.max-bucketsis anintoption whose description says it "should either be equal to -1 (unlimited), or it must be greater than 0" (CoreOptions:1694-1699). There is no upper bound in the description and no validation anywhere —git grep DYNAMIC_BUCKET_MAX_BUCKETSover main sources returns only the definition and the getter — so a value above 32767 is accepted as configured.Note 32767 itself is fine; the first affected id is 32768. My first attempt at a reproduction used
Short.MAX_VALUEas the bound and passed, which is worth mentioning in case anyone else probes this.I did not send a patch because the three fixes I can see all trade something, and the choice looks like yours:
Int2IntHashMapin the tree, so this means a new class, and it doubles the per-key memory of a structure that is deliberatelyshort— that cost lands on every dynamic-bucket table, including the vast majority that never exceed 32767 buckets.SchemaValidationwhendynamic-bucket.max-buckets > 32768. No memory cost, and it breaks no working configuration, since anything above that is already producing duplicate keys. But a job currently running with such a value would start failing to open the table rather than quietly misbehaving.globalMaxBucketIdtoShort.MAX_VALUEand log. Keeps such jobs running and stops the corruption, at the cost of silently giving fewer buckets than asked for.Happy to send whichever you prefer — I have the reproduction as a
PartitionIndexTestready to go with any of them.Are you willing to submit a PR?