Apache9 commented on code in PR #7653:
URL: https://github.com/apache/hbase/pull/7653#discussion_r2753309174
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java:
##########
@@ -153,16 +193,12 @@ public static BloomFilterWriter
createGeneralBloomAtWrite(Configuration conf,
return null;
}
- float err = getErrorRate(conf);
-
- // In case of row/column Bloom filter lookups, each lookup is an OR if two
- // separate lookups. Therefore, if each lookup's false positive rate is p,
- // the resulting false positive rate is err = 1 - (1 - p)^2, and
- // p = 1 - sqrt(1 - err).
- if (bloomType == BloomType.ROWCOL) {
- err = (float) (1 - Math.sqrt(1 - err));
+ // Check if Ribbon filter is requested
+ if (bloomImpl == BloomFilterImpl.RIBBON) {
+ return createRibbonFilterAtWrite(conf, cacheConf, bloomType, writer);
}
+ float err = (float) getAdjustedErrorRate(conf, bloomType);
Review Comment:
Also extract the below code to a method called createTraditionalBloomFilter?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java:
##########
@@ -338,7 +339,9 @@ private StoreContext
initializeStoreContext(ColumnFamilyDescriptor family) throw
return new StoreContext.Builder().withBlockSize(family.getBlocksize())
.withEncryptionContext(SecurityUtil.createEncryptionContext(conf,
region.getTableDescriptor(),
family, region.getManagedKeyDataCache(), region.getSystemKeyCache()))
-
.withBloomType(family.getBloomFilterType()).withCacheConfig(createCacheConf(family))
+ .withBloomType(family.getBloomFilterType())
+ .withBloomFilterImpl(BloomFilterFactory.getBloomFilterImpl(conf, family))
Review Comment:
Why here we can not call family.getBloomFilterImpl directly, like the above
for BloomType?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/util/BloomFilterFactory.java:
##########
@@ -153,16 +193,12 @@ public static BloomFilterWriter
createGeneralBloomAtWrite(Configuration conf,
return null;
}
- float err = getErrorRate(conf);
-
- // In case of row/column Bloom filter lookups, each lookup is an OR if two
- // separate lookups. Therefore, if each lookup's false positive rate is p,
- // the resulting false positive rate is err = 1 - (1 - p)^2, and
- // p = 1 - sqrt(1 - err).
- if (bloomType == BloomType.ROWCOL) {
- err = (float) (1 - Math.sqrt(1 - err));
+ // Check if Ribbon filter is requested
+ if (bloomImpl == BloomFilterImpl.RIBBON) {
Review Comment:
Better use switch case here? we could have other bloom implementation in the
future.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
