You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2021/11/13 01:32:26 UTC
[pinot] branch master updated: Add getName() to PartitionFunction interface (#7760)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 362f3e1 Add getName() to PartitionFunction interface (#7760)
362f3e1 is described below
commit 362f3e1acca6b50dcf2680848ac6dfec71d37d3b
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Fri Nov 12 17:32:10 2021 -0800
Add getName() to PartitionFunction interface (#7760)
---
.../realtime/PinotLLCRealtimeSegmentManager.java | 2 +-
.../helix/core/util/ZKMetadataUtils.java | 2 +-
.../indexsegment/mutable/MutableSegmentImpl.java | 2 +-
.../creator/impl/SegmentColumnarIndexCreator.java | 2 +-
.../spi/partition/ByteArrayPartitionFunction.java | 6 +++
.../spi/partition/HashCodePartitionFunction.java | 6 +++
.../spi/partition/ModuloPartitionFunction.java | 6 +++
.../spi/partition/MurmurPartitionFunction.java | 6 +++
.../segment/spi/partition/PartitionFunction.java | 7 +++
.../spi/partition/PartitionFunctionFactory.java | 3 +-
.../spi/partition/PartitionFunctionTest.java | 50 ++++++++++++++--------
11 files changed, 67 insertions(+), 25 deletions(-)
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
index 16c7471..154ba5d 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/realtime/PinotLLCRealtimeSegmentManager.java
@@ -727,7 +727,7 @@ public class PinotLLCRealtimeSegmentManager {
PartitionFunction partitionFunction = columnMetadata.getPartitionFunction();
if (partitionFunction != null) {
ColumnPartitionMetadata columnPartitionMetadata =
- new ColumnPartitionMetadata(partitionFunction.toString(), partitionFunction.getNumPartitions(),
+ new ColumnPartitionMetadata(partitionFunction.getName(), partitionFunction.getNumPartitions(),
columnMetadata.getPartitions());
return new SegmentPartitionMetadata(Collections.singletonMap(entry.getKey(), columnPartitionMetadata));
}
diff --git a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/ZKMetadataUtils.java b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/ZKMetadataUtils.java
index b4ee793..bfd557f 100644
--- a/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/ZKMetadataUtils.java
+++ b/pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/util/ZKMetadataUtils.java
@@ -67,7 +67,7 @@ public class ZKMetadataUtils {
if (partitionFunction != null) {
ColumnPartitionMetadata columnPartitionMetadata =
- new ColumnPartitionMetadata(partitionFunction.toString(), partitionFunction.getNumPartitions(),
+ new ColumnPartitionMetadata(partitionFunction.getName(), partitionFunction.getNumPartitions(),
columnMetadata.getPartitions());
columnPartitionMap.put(column, columnPartitionMetadata);
}
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
index 35fdfce..31c65da 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/indexsegment/mutable/MutableSegmentImpl.java
@@ -422,7 +422,7 @@ public class MutableSegmentImpl implements MutableSegment {
public SegmentPartitionConfig getSegmentPartitionConfig() {
if (_partitionColumn != null) {
return new SegmentPartitionConfig(Collections.singletonMap(_partitionColumn,
- new ColumnPartitionConfig(_partitionFunction.toString(), _partitionFunction.getNumPartitions())));
+ new ColumnPartitionConfig(_partitionFunction.getName(), _partitionFunction.getNumPartitions())));
} else {
return null;
}
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java
index 476c4c1..6651e64 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java
@@ -761,7 +761,7 @@ public class SegmentColumnarIndexCreator implements SegmentCreator {
PartitionFunction partitionFunction = columnIndexCreationInfo.getPartitionFunction();
if (partitionFunction != null) {
- properties.setProperty(getKeyFor(column, PARTITION_FUNCTION), partitionFunction.toString());
+ properties.setProperty(getKeyFor(column, PARTITION_FUNCTION), partitionFunction.getName());
properties.setProperty(getKeyFor(column, NUM_PARTITIONS), columnIndexCreationInfo.getNumPartitions());
properties.setProperty(getKeyFor(column, PARTITION_VALUES), columnIndexCreationInfo.getPartitions());
}
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ByteArrayPartitionFunction.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ByteArrayPartitionFunction.java
index 3aac9de..fc52fb8 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ByteArrayPartitionFunction.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ByteArrayPartitionFunction.java
@@ -45,10 +45,16 @@ public class ByteArrayPartitionFunction implements PartitionFunction {
}
@Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
public int getNumPartitions() {
return _numPartitions;
}
+ // Keep it for backward-compatibility, use getName() instead
@Override
public String toString() {
return NAME;
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/HashCodePartitionFunction.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/HashCodePartitionFunction.java
index 4ef556d..211bc21 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/HashCodePartitionFunction.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/HashCodePartitionFunction.java
@@ -42,10 +42,16 @@ public class HashCodePartitionFunction implements PartitionFunction {
}
@Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
public int getNumPartitions() {
return _numPartitions;
}
+ // Keep it for backward-compatibility, use getName() instead
@Override
public String toString() {
return NAME;
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ModuloPartitionFunction.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ModuloPartitionFunction.java
index 9f9b09f..e2e1d32 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ModuloPartitionFunction.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/ModuloPartitionFunction.java
@@ -65,10 +65,16 @@ public class ModuloPartitionFunction implements PartitionFunction {
}
@Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
public int getNumPartitions() {
return _numPartitions;
}
+ // Keep it for backward-compatibility, use getName() instead
@Override
public String toString() {
return NAME;
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/MurmurPartitionFunction.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/MurmurPartitionFunction.java
index 7b3e1d7..dd566e0 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/MurmurPartitionFunction.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/MurmurPartitionFunction.java
@@ -47,10 +47,16 @@ public class MurmurPartitionFunction implements PartitionFunction {
}
@Override
+ public String getName() {
+ return NAME;
+ }
+
+ @Override
public int getNumPartitions() {
return _numPartitions;
}
+ // Keep it for backward-compatibility, use getName() instead
@Override
public String toString() {
return NAME;
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java
index 5076c20..d0e3508 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunction.java
@@ -29,6 +29,7 @@ import java.io.Serializable;
* with the same value are expected to produce the same result.
*/
public interface PartitionFunction extends Serializable {
+
/**
* Method to compute and return partition id for the given value.
*
@@ -38,6 +39,12 @@ public interface PartitionFunction extends Serializable {
int getPartition(Object value);
/**
+ * Returns the name of the partition function.
+ * @return Name of the partition function.
+ */
+ String getName();
+
+ /**
* Returns the total number of possible partitions.
* @return Number of possible partitions.
*/
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunctionFactory.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunctionFactory.java
index 0a3a8e9..85f8319 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunctionFactory.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/partition/PartitionFunctionFactory.java
@@ -20,7 +20,6 @@ package org.apache.pinot.segment.spi.partition;
import java.util.HashMap;
import java.util.Map;
-import javax.annotation.Nonnull;
/**
@@ -67,7 +66,7 @@ public class PartitionFunctionFactory {
// a custom partition function could be used in the realtime stream partitioning or offline segment partitioning.
// The PartitionFunctionFactory should be able to support these default implementations, as well as instantiate
// based on config
- public static PartitionFunction getPartitionFunction(@Nonnull String functionName, int numPartitions) {
+ public static PartitionFunction getPartitionFunction(String functionName, int numPartitions) {
PartitionFunctionType function = PartitionFunctionType.fromString(functionName);
switch (function) {
case Modulo:
diff --git a/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/partition/PartitionFunctionTest.java b/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/partition/PartitionFunctionTest.java
index 12e3330..400cd11 100644
--- a/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/partition/PartitionFunctionTest.java
+++ b/pinot-segment-spi/src/test/java/org/apache/pinot/segment/spi/partition/PartitionFunctionTest.java
@@ -18,11 +18,14 @@
*/
package org.apache.pinot.segment.spi.partition;
+import com.fasterxml.jackson.databind.JsonNode;
import java.util.Random;
-import org.testng.Assert;
+import org.apache.pinot.spi.utils.JsonUtils;
import org.testng.annotations.Test;
import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertTrue;
/**
@@ -54,31 +57,31 @@ public class PartitionFunctionTest {
String functionName = "MoDuLo";
PartitionFunction partitionFunction =
PartitionFunctionFactory.getPartitionFunction(functionName, expectedNumPartitions);
- Assert.assertEquals(partitionFunction.toString().toLowerCase(), functionName.toLowerCase());
- Assert.assertEquals(partitionFunction.getNumPartitions(), expectedNumPartitions);
+
+ testBasicProperties(partitionFunction, functionName, expectedNumPartitions);
// Test int values.
for (int j = 0; j < 1000; j++) {
int value = random.nextInt();
- Assert.assertEquals(partitionFunction.getPartition(value), (value % expectedNumPartitions));
+ assertEquals(partitionFunction.getPartition(value), (value % expectedNumPartitions));
}
// Test long values.
for (int j = 0; j < 1000; j++) {
long value = random.nextLong();
- Assert.assertEquals(partitionFunction.getPartition(value), (value % expectedNumPartitions));
+ assertEquals(partitionFunction.getPartition(value), (value % expectedNumPartitions));
}
// Test Integer represented as String.
for (int j = 0; j < 1000; j++) {
int value = random.nextInt();
- Assert.assertEquals(partitionFunction.getPartition(Integer.toString(value)), (value % expectedNumPartitions));
+ assertEquals(partitionFunction.getPartition(Integer.toString(value)), (value % expectedNumPartitions));
}
// Test Long represented as String.
for (int j = 0; j < 1000; j++) {
long value = random.nextLong();
- Assert.assertEquals(partitionFunction.getPartition(Long.toString(value)), (value % expectedNumPartitions));
+ assertEquals(partitionFunction.getPartition(Long.toString(value)), (value % expectedNumPartitions));
}
}
}
@@ -104,16 +107,15 @@ public class PartitionFunctionTest {
}
String functionName = "mUrmur";
-
PartitionFunction partitionFunction =
PartitionFunctionFactory.getPartitionFunction(functionName, expectedNumPartitions);
- Assert.assertEquals(partitionFunction.toString().toLowerCase(), functionName.toLowerCase());
- Assert.assertEquals(partitionFunction.getNumPartitions(), expectedNumPartitions);
+
+ testBasicProperties(partitionFunction, functionName, expectedNumPartitions);
for (int j = 0; j < 1000; j++) {
int value = random.nextInt();
String stringValue = Integer.toString(value);
- Assert.assertTrue(partitionFunction.getPartition(stringValue) < expectedNumPartitions,
+ assertTrue(partitionFunction.getPartition(stringValue) < expectedNumPartitions,
"Illegal: " + partitionFunction.getPartition(stringValue) + " " + expectedNumPartitions);
}
}
@@ -142,12 +144,12 @@ public class PartitionFunctionTest {
String functionName = "bYteArray";
PartitionFunction partitionFunction =
PartitionFunctionFactory.getPartitionFunction(functionName, expectedNumPartitions);
- Assert.assertEquals(partitionFunction.toString().toLowerCase(), functionName.toLowerCase());
- Assert.assertEquals(partitionFunction.getNumPartitions(), expectedNumPartitions);
+
+ testBasicProperties(partitionFunction, functionName, expectedNumPartitions);
for (int j = 0; j < 1000; j++) {
Integer value = random.nextInt();
- Assert.assertTrue(partitionFunction.getPartition(value) < expectedNumPartitions,
+ assertTrue(partitionFunction.getPartition(value) < expectedNumPartitions,
"Illegal: " + partitionFunction.getPartition(value) + " " + expectedNumPartitions);
}
}
@@ -169,16 +171,26 @@ public class PartitionFunctionTest {
String functionName = "HaShCoDe";
PartitionFunction partitionFunction =
PartitionFunctionFactory.getPartitionFunction(functionName, expectedNumPartitions);
- Assert.assertEquals(partitionFunction.toString().toLowerCase(), functionName.toLowerCase());
- Assert.assertEquals(partitionFunction.getNumPartitions(), expectedNumPartitions);
+
+ testBasicProperties(partitionFunction, functionName, expectedNumPartitions);
for (int j = 0; j < 1000; j++) {
Integer value = random.nextInt();
- Assert.assertEquals(partitionFunction.getPartition(value), Math.abs(value.hashCode()) % expectedNumPartitions);
+ assertEquals(partitionFunction.getPartition(value), Math.abs(value.hashCode()) % expectedNumPartitions);
}
}
}
+ private void testBasicProperties(PartitionFunction partitionFunction, String functionName, int numPartitions) {
+ assertEquals(partitionFunction.getName().toLowerCase(), functionName.toLowerCase());
+ assertEquals(partitionFunction.getNumPartitions(), numPartitions);
+
+ JsonNode jsonNode = JsonUtils.objectToJsonNode(partitionFunction);
+ assertEquals(jsonNode.size(), 2);
+ assertEquals(jsonNode.get("name").asText().toLowerCase(), functionName.toLowerCase());
+ assertEquals(jsonNode.get("numPartitions").asInt(), numPartitions);
+ }
+
/**
* Tests the equivalence of org.apache.kafka.common.utils.Utils::murmur2 and {@link MurmurPartitionFunction::murmur2}
* Our implementation of murmur2 has been copied over from Utils::murmur2
@@ -206,7 +218,7 @@ public class PartitionFunctionTest {
for (int expectedMurmurValue : expectedMurmurValues) {
random.nextBytes(array);
int actualMurmurValue = murmurPartitionFunction.murmur2(array);
- Assert.assertEquals(actualMurmurValue, expectedMurmurValue);
+ assertEquals(actualMurmurValue, expectedMurmurValue);
}
}
@@ -265,7 +277,7 @@ public class PartitionFunctionTest {
random.nextBytes(array);
String nextString = new String(array, UTF_8);
int actualPartition = partitionFunction.getPartition(nextString);
- Assert.assertEquals(actualPartition, expectedPartition);
+ assertEquals(actualPartition, expectedPartition);
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org