PDGGK opened a new issue, #9276:
URL: https://github.com/apache/paimon/issues/9276
### Search before asking
- [x] I searched in the issues and found nothing similar.
### Paimon version
master (`a70912b`)
### Compute Engine
Flink, dynamic-bucket primary-key table. Reproduced as a unit test, so
engine-independent.
### Minimal reproduce step
```java
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
```java
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:
```java
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:**
1. **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.
2. **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.
3. **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?
- [x] I'm willing to submit a PR!
--
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]
