Skip to content

Commit 1329e1f

Browse files
xiangfu0claude
andauthored
Add useRawBytes config to Murmur and Murmur3 partition functions (#17932)
* Add useRawBytes config to Murmur and Murmur3 partition functions When partitioning on BYTES columns, the partition value is hex-encoded. With useRawBytes=true in functionConfig, the hex string is decoded back to raw bytes before hashing, ensuring partition assignment matches the original byte values rather than treating the hex string as UTF-8 text. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Override getFunctionConfig() and restore backward-compatible constructor - Murmur and Murmur3 partition functions now persist and return the functionConfig via getFunctionConfig(), ensuring partition metadata written to ZK retains the config so brokers can reconstruct the function correctly. - Re-introduce MurmurPartitionFunction(int) constructor delegating to the new (int, Map) constructor to preserve SPI backward compatibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Defensive copy and @nullable annotations for functionConfig - Store functionConfig as Collections.unmodifiableMap() to prevent external mutation from diverging with parsed fields. - Annotate _functionConfig field and getFunctionConfig() with @nullable in both MurmurPartitionFunction and Murmur3PartitionFunction. - Annotate Murmur3PartitionFunction constructor parameter with @nullable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add factory end-to-end test for useRawBytes with Murmur/Murmur2/Murmur3 Verify that PartitionFunctionFactory.getPartitionFunction() correctly wires functionConfig through to the partition function for all three aliases, and that getFunctionConfig() roundtrips the config. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fc95b79 commit 1329e1f

4 files changed

Lines changed: 148 additions & 4 deletions

File tree

pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/Murmur3PartitionFunction.java

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@
1919
package org.apache.pinot.segment.spi.partition;
2020

2121
import com.google.common.base.Preconditions;
22+
import java.util.Collections;
2223
import java.util.Map;
24+
import javax.annotation.Nullable;
2325
import org.apache.commons.lang3.StringUtils;
26+
import org.apache.pinot.spi.utils.BytesUtils;
2427
import org.apache.pinot.spi.utils.hash.MurmurHashFunctions;
2528

2629
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -33,21 +36,27 @@ public class Murmur3PartitionFunction implements PartitionFunction {
3336
private static final String NAME = "Murmur3";
3437
private static final String SEED_KEY = "seed";
3538
private static final String VARIANT_KEY = "variant";
39+
private static final String USE_RAW_BYTES_KEY = "useRawBytes";
3640
private final int _numPartitions;
41+
@Nullable
42+
private final Map<String, String> _functionConfig;
3743
private final int _seed;
3844
private final boolean _useX64;
45+
private final boolean _useRawBytes;
3946

4047
/**
4148
* Constructor for the class.
4249
* @param numPartitions Number of partitions.
4350
* @param functionConfig to extract configurations for the partition function.
4451
*/
45-
public Murmur3PartitionFunction(int numPartitions, Map<String, String> functionConfig) {
52+
public Murmur3PartitionFunction(int numPartitions, @Nullable Map<String, String> functionConfig) {
4653
Preconditions.checkArgument(numPartitions > 0, "Number of partitions must be > 0");
4754
_numPartitions = numPartitions;
55+
_functionConfig = functionConfig != null ? Collections.unmodifiableMap(functionConfig) : null;
4856

4957
int seed = 0;
5058
boolean useX64 = false;
59+
boolean useRawBytes = false;
5160
if (functionConfig != null) {
5261
String seedString = functionConfig.get(SEED_KEY);
5362
if (StringUtils.isNotEmpty(seedString)) {
@@ -62,13 +71,21 @@ public Murmur3PartitionFunction(int numPartitions, Map<String, String> functionC
6271
"Murmur3 variant must be either x86_32 or x64_32");
6372
}
6473
}
74+
useRawBytes = Boolean.parseBoolean(functionConfig.get(USE_RAW_BYTES_KEY));
6575
}
6676
_seed = seed;
6777
_useX64 = useX64;
78+
_useRawBytes = useRawBytes;
6879
}
6980

7081
@Override
7182
public int getPartition(String value) {
83+
if (_useRawBytes) {
84+
byte[] bytes = BytesUtils.toBytes(value);
85+
int hash = _useX64 ? MurmurHashFunctions.murmurHash3X64Bit32(bytes, _seed)
86+
: MurmurHashFunctions.murmurHash3X86Bit32(bytes, _seed);
87+
return (hash & Integer.MAX_VALUE) % _numPartitions;
88+
}
7289
int hash = _useX64 ? MurmurHashFunctions.murmurHash3X64Bit32(value, _seed)
7390
: MurmurHashFunctions.murmurHash3X86Bit32(value.getBytes(UTF_8), _seed);
7491
return (hash & Integer.MAX_VALUE) % _numPartitions;
@@ -84,6 +101,12 @@ public int getNumPartitions() {
84101
return _numPartitions;
85102
}
86103

104+
@Nullable
105+
@Override
106+
public Map<String, String> getFunctionConfig() {
107+
return _functionConfig;
108+
}
109+
87110
// Keep it for backward-compatibility, use getName() instead
88111
@Override
89112
public String toString() {

pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/MurmurPartitionFunction.java

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
package org.apache.pinot.segment.spi.partition;
2020

2121
import com.google.common.base.Preconditions;
22+
import java.util.Collections;
23+
import java.util.Map;
24+
import javax.annotation.Nullable;
25+
import org.apache.pinot.spi.utils.BytesUtils;
2226
import org.apache.pinot.spi.utils.hash.MurmurHashFunctions;
2327

2428
import static java.nio.charset.StandardCharsets.UTF_8;
@@ -29,20 +33,36 @@
2933
*/
3034
public class MurmurPartitionFunction implements PartitionFunction {
3135
private static final String NAME = "Murmur";
36+
private static final String USE_RAW_BYTES_KEY = "useRawBytes";
3237
private final int _numPartitions;
38+
@Nullable
39+
private final Map<String, String> _functionConfig;
40+
private final boolean _useRawBytes;
3341

3442
/**
35-
* Constructor for the class.
43+
* Constructor for backward compatibility.
3644
* @param numPartitions Number of partitions.
3745
*/
3846
public MurmurPartitionFunction(int numPartitions) {
47+
this(numPartitions, null);
48+
}
49+
50+
/**
51+
* Constructor for the class.
52+
* @param numPartitions Number of partitions.
53+
* @param functionConfig to extract configurations for the partition function.
54+
*/
55+
public MurmurPartitionFunction(int numPartitions, @Nullable Map<String, String> functionConfig) {
3956
Preconditions.checkArgument(numPartitions > 0, "Number of partitions must be > 0");
4057
_numPartitions = numPartitions;
58+
_functionConfig = functionConfig != null ? Collections.unmodifiableMap(functionConfig) : null;
59+
_useRawBytes = functionConfig != null && Boolean.parseBoolean(functionConfig.get(USE_RAW_BYTES_KEY));
4160
}
4261

4362
@Override
4463
public int getPartition(String value) {
45-
return (MurmurHashFunctions.murmurHash2(value.getBytes(UTF_8)) & Integer.MAX_VALUE) % _numPartitions;
64+
byte[] bytes = _useRawBytes ? BytesUtils.toBytes(value) : value.getBytes(UTF_8);
65+
return (MurmurHashFunctions.murmurHash2(bytes) & Integer.MAX_VALUE) % _numPartitions;
4666
}
4767

4868
@Override
@@ -55,6 +75,12 @@ public int getNumPartitions() {
5575
return _numPartitions;
5676
}
5777

78+
@Nullable
79+
@Override
80+
public Map<String, String> getFunctionConfig() {
81+
return _functionConfig;
82+
}
83+
5884
// Keep it for backward-compatibility, use getName() instead
5985
@Override
6086
public String toString() {

pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunctionFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ public static PartitionFunction getPartitionFunction(String functionName, int nu
7777

7878
case Murmur:
7979
case Murmur2:
80-
return new MurmurPartitionFunction(numPartitions);
80+
return new MurmurPartitionFunction(numPartitions, functionConfig);
8181

8282
case Murmur3:
8383
return new Murmur3PartitionFunction(numPartitions, functionConfig);

pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/partition/PartitionFunctionTest.java

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.HashMap;
2323
import java.util.Map;
2424
import java.util.Random;
25+
import org.apache.pinot.spi.utils.BytesUtils;
2526
import org.apache.pinot.spi.utils.JsonUtils;
2627
import org.apache.pinot.spi.utils.hash.MurmurHashFunctions;
2728
import org.testng.annotations.Test;
@@ -439,6 +440,100 @@ public void testMurmurPartitionFunctionEquivalence() {
439440
testPartitionFunction(murmurPartitionFunction, expectedPartitions);
440441
}
441442

443+
@Test
444+
public void testMurmurPartitionFunctionUseRawBytes() {
445+
int numPartitions = 5;
446+
Map<String, String> functionConfig = new HashMap<>();
447+
functionConfig.put("useRawBytes", "true");
448+
MurmurPartitionFunction partitionFunction = new MurmurPartitionFunction(numPartitions, functionConfig);
449+
450+
// When useRawBytes is true, the hex-encoded value should be decoded back to raw bytes before hashing.
451+
// This means getPartition(hexString) should produce the same result as hashing the original raw bytes.
452+
byte[] rawBytes = new byte[]{0x01, 0x02, 0x03, 0x04, 0x05};
453+
String hexValue = BytesUtils.toHexString(rawBytes);
454+
455+
int expectedPartition =
456+
(MurmurHashFunctions.murmurHash2(rawBytes) & Integer.MAX_VALUE) % numPartitions;
457+
assertEquals(partitionFunction.getPartition(hexValue), expectedPartition);
458+
459+
// Without useRawBytes, the same hex string should be treated as UTF-8 text (different result).
460+
MurmurPartitionFunction defaultPartitionFunction = new MurmurPartitionFunction(numPartitions, null);
461+
int defaultPartition =
462+
(MurmurHashFunctions.murmurHash2(hexValue.getBytes(UTF_8)) & Integer.MAX_VALUE) % numPartitions;
463+
assertEquals(defaultPartitionFunction.getPartition(hexValue), defaultPartition);
464+
}
465+
466+
@Test
467+
public void testMurmur3PartitionFunctionUseRawBytes() {
468+
int numPartitions = 5;
469+
byte[] rawBytes = new byte[]{0x01, 0x02, 0x03, 0x04, 0x05};
470+
String hexValue = BytesUtils.toHexString(rawBytes);
471+
472+
// Test x86_32 variant with useRawBytes
473+
Map<String, String> functionConfig = new HashMap<>();
474+
functionConfig.put("useRawBytes", "true");
475+
Murmur3PartitionFunction partitionFunction = new Murmur3PartitionFunction(numPartitions, functionConfig);
476+
477+
int expectedPartition =
478+
(MurmurHashFunctions.murmurHash3X86Bit32(rawBytes, 0) & Integer.MAX_VALUE) % numPartitions;
479+
assertEquals(partitionFunction.getPartition(hexValue), expectedPartition);
480+
481+
// Test x64_32 variant with useRawBytes
482+
functionConfig.put("variant", "x64_32");
483+
Murmur3PartitionFunction partitionFunctionX64 = new Murmur3PartitionFunction(numPartitions, functionConfig);
484+
485+
int expectedPartitionX64 =
486+
(MurmurHashFunctions.murmurHash3X64Bit32(rawBytes, 0) & Integer.MAX_VALUE) % numPartitions;
487+
assertEquals(partitionFunctionX64.getPartition(hexValue), expectedPartitionX64);
488+
489+
// Test x86_32 variant with useRawBytes and non-zero seed
490+
functionConfig.remove("variant");
491+
functionConfig.put("seed", "9001");
492+
Murmur3PartitionFunction partitionFunctionWithSeed = new Murmur3PartitionFunction(numPartitions, functionConfig);
493+
494+
int expectedPartitionWithSeed =
495+
(MurmurHashFunctions.murmurHash3X86Bit32(rawBytes, 9001) & Integer.MAX_VALUE) % numPartitions;
496+
assertEquals(partitionFunctionWithSeed.getPartition(hexValue), expectedPartitionWithSeed);
497+
498+
// Without useRawBytes, the same hex string should be treated as UTF-8 text.
499+
Murmur3PartitionFunction defaultPartitionFunction = new Murmur3PartitionFunction(numPartitions, null);
500+
int defaultPartition =
501+
(MurmurHashFunctions.murmurHash3X86Bit32(hexValue.getBytes(UTF_8), 0) & Integer.MAX_VALUE) % numPartitions;
502+
assertEquals(defaultPartitionFunction.getPartition(hexValue), defaultPartition);
503+
}
504+
505+
@Test
506+
public void testUseRawBytesThroughFactory() {
507+
int numPartitions = 5;
508+
byte[] rawBytes = new byte[]{0x01, 0x02, 0x03, 0x04, 0x05};
509+
String hexValue = BytesUtils.toHexString(rawBytes);
510+
int expectedPartition =
511+
(MurmurHashFunctions.murmurHash2(rawBytes) & Integer.MAX_VALUE) % numPartitions;
512+
513+
Map<String, String> functionConfig = new HashMap<>();
514+
functionConfig.put("useRawBytes", "true");
515+
516+
// Test Murmur alias through factory
517+
PartitionFunction murmurFn =
518+
PartitionFunctionFactory.getPartitionFunction("Murmur", numPartitions, functionConfig);
519+
assertEquals(murmurFn.getPartition(hexValue), expectedPartition);
520+
assertEquals(murmurFn.getFunctionConfig().get("useRawBytes"), "true");
521+
522+
// Test Murmur2 alias through factory
523+
PartitionFunction murmur2Fn =
524+
PartitionFunctionFactory.getPartitionFunction("Murmur2", numPartitions, functionConfig);
525+
assertEquals(murmur2Fn.getPartition(hexValue), expectedPartition);
526+
assertEquals(murmur2Fn.getFunctionConfig().get("useRawBytes"), "true");
527+
528+
// Test Murmur3 through factory
529+
int expectedPartitionMurmur3 =
530+
(MurmurHashFunctions.murmurHash3X86Bit32(rawBytes, 0) & Integer.MAX_VALUE) % numPartitions;
531+
PartitionFunction murmur3Fn =
532+
PartitionFunctionFactory.getPartitionFunction("Murmur3", numPartitions, functionConfig);
533+
assertEquals(murmur3Fn.getPartition(hexValue), expectedPartitionMurmur3);
534+
assertEquals(murmur3Fn.getFunctionConfig().get("useRawBytes"), "true");
535+
}
536+
442537
@Test
443538
public void testMurmur3PartitionFunctionEquivalence() {
444539

0 commit comments

Comments
 (0)