taklwu commented on a change in pull request #2800:
URL: https://github.com/apache/hbase/pull/2800#discussion_r546951790



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -474,33 +501,14 @@ public long getBlockingFileCount() {
   }
   /* End implementation of StoreConfigInformation */
 
-  /**
-   * Returns the configured bytesPerChecksum value.
-   * @param conf The configuration
-   * @return The bytesPerChecksum that is set in the configuration
-   */
-  public static int getBytesPerChecksum(Configuration conf) {
-    return conf.getInt(HConstants.BYTES_PER_CHECKSUM,
-                       HFile.DEFAULT_BYTES_PER_CHECKSUM);
-  }
-
-  /**
-   * Returns the configured checksum algorithm.
-   * @param conf The configuration
-   * @return The checksum algorithm that is set in the configuration
-   */
-  public static ChecksumType getChecksumType(Configuration conf) {
-    String checksumName = conf.get(HConstants.CHECKSUM_TYPE_NAME);
-    if (checksumName == null) {
-      return ChecksumType.getDefaultChecksumType();
-    } else {
-      return ChecksumType.nameToType(checksumName);
-    }
-  }
 
   @Override
   public ColumnFamilyDescriptor getColumnFamilyDescriptor() {
-    return this.family;
+    return this.storeContext.getFamily();
+  }
+
+  public Encryption.Context getCryptoContext() {

Review comment:
       sure, and thanks for pointing it out

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -2559,7 +2572,7 @@ public boolean needsCompaction() {
    * @return cache configuration for this Store.
    */
   public CacheConfig getCacheConfig() {
-    return this.cacheConf;
+    return storeContext.getCacheConf();
   }
 
   public static final long FIXED_OVERHEAD = 
ClassSize.estimateBase(HStore.class, false);

Review comment:
       TestHeapSize still passed when I checked it locally.
   
   from what I understood from the `FIXED_OVERHEAD` were being calculated by 
`ClassSize.estimateBase`, it will automatically calculate the our newly added 
fields and reference included the change of `HStoreContext storeContext`. 
   
   So for the `DEEP_OVERHEAD` that takes new calculated `FIXED_OVERHEAD` to 
come up the heap size, it should be done already. Or did I miss something here? 

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStoreContext.java
##########
@@ -0,0 +1,174 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.hbase.CellComparator;
+import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
+import org.apache.hadoop.hbase.io.hfile.CacheConfig;
+import org.apache.hadoop.hbase.io.hfile.HFileContext;
+import org.apache.yetus.audience.InterfaceAudience;
+
+import java.net.InetSocketAddress;
+import java.util.Collection;
+import java.util.function.Supplier;
+
+/**
+ * This carries the information on some of the meta data about the HStore. This
+ * meta data can be used across the HFileWriter/Readers and other HStore 
consumers without the
+ * need of passing around the complete store.
+ */
[email protected]
+public class HStoreContext {

Review comment:
       as I mentioned above about `FIXED_OVERHEAD` were being calculated by 
`ClassSize.estimateBase`, it should be already covered 




----------------------------------------------------------------
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:
[email protected]


Reply via email to