[GitHub] [incubator-hudi] smarthi commented on a change in pull request #1253: [HUDI-558] Introduce ability to compress bloom filters while storing in parquet

2020-01-20 Thread GitBox 
smarthi commented on a change in pull request #1253: [HUDI-558] Introduce 
ability to compress bloom filters while storing in parquet
URL: https://github.com/apache/incubator-hudi/pull/1253#discussion_r368711970
 
 

 ##
 File path: hudi-cli/src/main/scala/org/apache/hudi/cli/SparkHelpers.scala
 ##
 @@ -43,7 +43,7 @@ object SparkHelpers {
 val schema: Schema = sourceRecords.get(0).getSchema
 val filter: BloomFilter = 
BloomFilterFactory.createBloomFilter(HoodieIndexConfig.DEFAULT_BLOOM_FILTER_NUM_ENTRIES.toInt,
 HoodieIndexConfig.DEFAULT_BLOOM_FILTER_FPP.toDouble,
   
HoodieIndexConfig.DEFAULT_HOODIE_BLOOM_INDEX_FILTER_DYNAMIC_MAX_ENTRIES.toInt, 
HoodieIndexConfig.DEFAULT_BLOOM_INDEX_FILTER_TYPE);
-val writeSupport: HoodieAvroWriteSupport = new HoodieAvroWriteSupport(new 
AvroSchemaConverter().convert(schema), schema, filter)
+val writeSupport: HoodieAvroWriteSupport = new HoodieAvroWriteSupport(new 
AvroSchemaConverter().convert(schema), schema, filter, 
java.lang.Boolean.valueOf(HoodieIndexConfig.BLOOM_INDEX_ENABLE_COMPRESSION))
 
 Review comment:
   replace Boolean.valueOf() with Boolean.parseBoolean().


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 #1253: [HUDI-558] Introduce ability to compress bloom filters while storing in parquet

2020-01-20 Thread GitBox 
smarthi commented on a change in pull request #1253: [HUDI-558] Introduce 
ability to compress bloom filters while storing in parquet
URL: https://github.com/apache/incubator-hudi/pull/1253#discussion_r368711264
 
 

 ##
 File path: 
hudi-client/src/main/java/org/apache/hudi/config/HoodieIndexConfig.java
 ##
 @@ -166,6 +168,11 @@ public Builder bloomIndexBucketizedChecking(boolean 
bucketizedChecking) {
   return this;
 }
 
+public Builder bloomIndexEnableCompression(boolean enableCompression) {
+  props.setProperty(BLOOM_INDEX_ENABLE_COMPRESSION, 
String.valueOf(enableCompression));
 
 Review comment:
   just call Boolean.toString() instead of String.valueOf()


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 #1253: [HUDI-558] Introduce ability to compress bloom filters while storing in parquet

2020-01-20 Thread GitBox 
smarthi commented on a change in pull request #1253: [HUDI-558] Introduce 
ability to compress bloom filters while storing in parquet
URL: https://github.com/apache/incubator-hudi/pull/1253#discussion_r368710257
 
 

 ##
 File path: 
hudi-common/src/main/java/org/apache/hudi/common/util/ParquetUtils.java
 ##
 @@ -149,13 +150,26 @@ public static BloomFilter 
readBloomFilterFromParquetMetadata(Configuration confi
 readParquetFooter(configuration, false, parquetFilePath,
 HoodieAvroWriteSupport.HOODIE_AVRO_BLOOM_FILTER_METADATA_KEY,
 HoodieAvroWriteSupport.OLD_HOODIE_AVRO_BLOOM_FILTER_METADATA_KEY,
-HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_TYPE_CODE);
+HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_TYPE_CODE,
+HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_IS_COMPRESSED,
+HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_COMPRESSION_TYPE);
 String footerVal = 
footerVals.get(HoodieAvroWriteSupport.HOODIE_AVRO_BLOOM_FILTER_METADATA_KEY);
 if (null == footerVal) {
   // We use old style key "com.uber.hoodie.bloomfilter"
   footerVal = 
footerVals.get(HoodieAvroWriteSupport.OLD_HOODIE_AVRO_BLOOM_FILTER_METADATA_KEY);
 }
 BloomFilter toReturn = null;
+boolean isCompressed = false;
+if 
(footerVals.containsKey(HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_IS_COMPRESSED))
 {
+  isCompressed = 
Boolean.valueOf(footerVals.get(HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_IS_COMPRESSED));
+  if (isCompressed) {
+String compressionType = 
footerVals.get(HoodieAvroWriteSupport.HOODIE_BLOOM_FILTER_COMPRESSION_TYPE);
+
Preconditions.checkArgument(compressionType.equals(GzipCompressionUtils.TYPE),
 
 Review comment:
   this can be replaced with ValidationUtils.checkArgument() once the PR# 1159 
has been merged 


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 #1253: [HUDI-558] Introduce ability to compress bloom filters while storing in parquet

2020-01-20 Thread GitBox 
smarthi commented on a change in pull request #1253: [HUDI-558] Introduce 
ability to compress bloom filters while storing in parquet
URL: https://github.com/apache/incubator-hudi/pull/1253#discussion_r368708574
 
 

 ##
 File path: 
hudi-client/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java
 ##
 @@ -318,6 +318,10 @@ public double getBloomFilterFPP() {
 return 
Double.parseDouble(props.getProperty(HoodieIndexConfig.BLOOM_FILTER_FPP));
   }
 
+  public boolean isBloomFilterCompressionEnabled() {
+return 
Boolean.valueOf(props.getProperty(HoodieIndexConfig.BLOOM_INDEX_ENABLE_COMPRESSION));
 
 Review comment:
   use Boolean.parseBoolean() instead ??


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