[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r392467151
##
File path:
hudi-client/src/main/java/org/apache/hudi/index/bloom/BucketizedBloomCheckPartitioner.java
##
@@ -144,7 +144,12 @@ public int numPartitions() {
@Override
public int getPartition(Object key) {
final Pair parts = (Pair) key;
-final long hashOfKey = Hashing.md5().hashString(parts.getRight(),
StandardCharsets.UTF_8).asLong();
+long hashOfKey = 0L;
Review comment:
Reverting back to MD5
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r392151737
##
File path:
hudi-client/src/main/java/org/apache/hudi/index/bloom/BucketizedBloomCheckPartitioner.java
##
@@ -144,7 +144,12 @@ public int numPartitions() {
@Override
public int getPartition(Object key) {
final Pair parts = (Pair) key;
-final long hashOfKey = Hashing.md5().hashString(parts.getRight(),
StandardCharsets.UTF_8).asLong();
+long hashOfKey = 0L;
Review comment:
replaced with SHA-256 - but got'ta test it out on long strings to see how it
works.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r392211319
##
File path:
hudi-client/src/main/java/org/apache/hudi/index/bloom/BucketizedBloomCheckPartitioner.java
##
@@ -144,7 +144,12 @@ public int numPartitions() {
@Override
public int getPartition(Object key) {
final Pair parts = (Pair) key;
-final long hashOfKey = Hashing.md5().hashString(parts.getRight(),
StandardCharsets.UTF_8).asLong();
+long hashOfKey = 0L;
Review comment:
CI tests passed with SHA-256.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r392154852
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/view/HoodieTableFileSystemView.java
##
@@ -142,7 +142,7 @@ protected void
resetPendingCompactionOperations(Stream> operations) {
operations.forEach(opInstantPair -> {
-
Preconditions.checkArgument(!fgIdToPendingCompaction.containsKey(opInstantPair.getValue().getFileGroupId()),
+
ValidationUtils.checkArgument(!fgIdToPendingCompaction.containsKey(opInstantPair.getValue().getFileGroupId()),
Review comment:
where is it?
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r392151737
##
File path:
hudi-client/src/main/java/org/apache/hudi/index/bloom/BucketizedBloomCheckPartitioner.java
##
@@ -144,7 +144,12 @@ public int numPartitions() {
@Override
public int getPartition(Object key) {
final Pair parts = (Pair) key;
-final long hashOfKey = Hashing.md5().hashString(parts.getRight(),
StandardCharsets.UTF_8).asLong();
+long hashOfKey = 0L;
Review comment:
replaced with SHA-256 - but got'ta test it out on long strings to see how it
works.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r384312049 ## File path: hudi-client/src/main/java/org/apache/hudi/io/compact/HoodieMergeOnReadTableCompactor.java ## @@ -18,6 +18,7 @@ package org.apache.hudi.io.compact; +import com.google.common.collect.Sets; Review comment: done 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r382958076
##
File path:
hudi-common/src/test/java/org/apache/hudi/common/minicluster/HdfsTestService.java
##
@@ -66,7 +66,7 @@ public Configuration getHadoopConf() {
}
public MiniDFSCluster start(boolean format) throws IOException {
-Preconditions.checkState(workDir != null, "The work dir must be set before
starting cluster.");
+Objects.requireNonNull(workDir, "The work dir must be set before starting
cluster.");
Review comment:
For this null check, its unnecessary - there is a checkState in
ValidationUtils to check other boolean conditions. But null check, its fine to
use Objects.checkNotNull()
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r382958083
##
File path:
hudi-hive/src/test/java/org/apache/hudi/hive/util/HiveTestService.java
##
@@ -87,7 +87,7 @@ public Configuration getHadoopConf() {
}
public HiveServer2 start() throws IOException {
-Preconditions.checkState(workDir != null, "The work dir must be set before
starting cluster.");
+Objects.requireNonNull(workDir, "The work dir must be set before starting
cluster.");
Review comment:
ditto
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r382958076
##
File path:
hudi-common/src/test/java/org/apache/hudi/common/minicluster/HdfsTestService.java
##
@@ -66,7 +66,7 @@ public Configuration getHadoopConf() {
}
public MiniDFSCluster start(boolean format) throws IOException {
-Preconditions.checkState(workDir != null, "The work dir must be set before
starting cluster.");
+Objects.requireNonNull(workDir, "The work dir must be set before starting
cluster.");
Review comment:
For this null check, its unnecessary - there is a checkState in
ValidationUtils to check other boolean conditions. But null check, its fine to
use Bojects.checkNotNull()
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1350: [HUDI-629]: Replace Guava's Hashing with an equivalent in NumericUtils.java
smarthi commented on a change in pull request #1350: [HUDI-629]: Replace
Guava's Hashing with an equivalent in NumericUtils.java
URL: https://github.com/apache/incubator-hudi/pull/1350#discussion_r382957997
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/util/NumericUtils.java
##
@@ -31,4 +41,28 @@ public static String humanReadableByteCount(double bytes) {
String pre = "KMGTPE".charAt(exp - 1) + "";
return String.format("%.1f %sB", bytes / Math.pow(1024, exp), pre);
}
+
+ public static long getMessageDigestHash(final String algorithmName, final
String string) {
+MessageDigest md = null;
+try {
+ md = MessageDigest.getInstance(algorithmName);
+} catch (NoSuchAlgorithmException e) {
+ LOGGER.error("Invalid Algorithm Specified: {}", algorithmName);
Review comment:
ok will do
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
