Skip to content

[Bug] PartitionIndex narrows the bucket id to short, so dynamic-bucket.max-buckets above 32767 sends one key to two buckets #9276

Description

@PDGGK

Search before asking

  • 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

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:

  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?

  • I'm willing to submit a PR!

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions