You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "ankitsultana (via GitHub)" <gi...@apache.org> on 2024/05/16 21:16:15 UTC

[PR] Add Support for Pluggable Hash Functions for Upsert Primary Keys [pinot]

ankitsultana opened a new pull request, #13170:
URL: https://github.com/apache/pinot/pull/13170

   TODO


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add Support for Pluggable Hash Functions for Upsert Primary Keys [pinot]

Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #13170:
URL: https://github.com/apache/pinot/pull/13170#discussion_r1604084141


##########
pinot-common/src/main/java/org/apache/pinot/common/upsert/hash/UpsertHashFunctionFactory.java:
##########
@@ -0,0 +1,69 @@
+package org.apache.pinot.common.upsert.hash;
+
+import java.lang.reflect.Constructor;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import org.apache.pinot.spi.utils.PinotReflectionUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+
+public class UpsertHashFunctionFactory {
+  private static final Logger LOGGER = LoggerFactory.getLogger(UpsertHashFunctionFactory.class);
+  private static final Map<String, Class<UpsertHashFunction>> HASH_FUNCTION_MAP = new HashMap<>();
+  private static final Object INIT_LOCK = new Object();
+  private volatile static boolean _isLoaded = false;
+
+  private UpsertHashFunctionFactory() {
+  }
+
+  public static UpsertHashFunction create(String functionName) {
+    if (!_isLoaded) {

Review Comment:
   Ideally we want to load all functions upfront to guarantee performance at ingestion time. It can also simply the locks. See `FunctionRegistry` for example. We may init it in `ServerInstance`



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add Support for Pluggable Hash Functions for Upsert Primary Keys [pinot]

Posted by "ankitsultana (via GitHub)" <gi...@apache.org>.
ankitsultana commented on code in PR #13170:
URL: https://github.com/apache/pinot/pull/13170#discussion_r1604053220


##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/upsert/ConcurrentMapPartitionUpsertMetadataManagerTest.java:
##########
@@ -153,12 +157,12 @@ public void testStartFinishOperation() {
   @Test
   public void testAddReplaceRemoveSegment()
       throws IOException {
-    verifyAddReplaceRemoveSegment(HashFunction.NONE, false);
-    verifyAddReplaceRemoveSegment(HashFunction.MD5, false);
-    verifyAddReplaceRemoveSegment(HashFunction.MURMUR3, false);
-    verifyAddReplaceRemoveSegment(HashFunction.NONE, true);
-    verifyAddReplaceRemoveSegment(HashFunction.MD5, true);
-    verifyAddReplaceRemoveSegment(HashFunction.MURMUR3, true);
+    verifyAddReplaceRemoveSegment(NoOpHashFunction.INSTANCE, false);
+    verifyAddReplaceRemoveSegment(MD5UpsertHashFunction.INSTANCE, false);
+    verifyAddReplaceRemoveSegment(Murmur3UpsertHashFunction.INSTANCE, false);

Review Comment:
   self-review: forgot to update the test `testHashPrimaryKeys`. We can also remove `HashUtils`.



-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add Support for Pluggable Hash Functions for Upsert Primary Keys [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #13170:
URL: https://github.com/apache/pinot/pull/13170#issuecomment-4746474032

   Closing this PR as stale because it has had no activity for more than one year (last updated before 2025-06-18). If this is still relevant, please reopen it or open a fresh PR against current master. Thanks for the contribution.


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org


Re: [PR] Add Support for Pluggable Hash Functions for Upsert Primary Keys [pinot]

Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 closed pull request #13170: Add Support for Pluggable Hash Functions for Upsert Primary Keys
URL: https://github.com/apache/pinot/pull/13170


-- 
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: commits-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org