You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "jojochuang (via GitHub)" <gi...@apache.org> on 2024/03/14 21:32:42 UTC

[PR] Hdds 9842 [ozone]

jojochuang opened a new pull request, #6381:
URL: https://github.com/apache/ozone/pull/6381

   ## What changes were proposed in this pull request?
   HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase.
   
   Volume space usage is checked for every write request (WriteChunk, PutBlock). It's okay for large payloads but for HBase WAL where a client sends thousands of write requests per second, this overhead is unacceptable.
   
   Proposed solution: cache volume space usage using Guava Cache. Cache is refreshed periodically every minute. The cached value is used to determine if the volume is full or not, and its value doesn't need to be accurate. One minute staleness should be acceptable. If not, even 1 second interval should be good enough.
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-9842
   
   ## How was this patch tested?
   Existing unit tests


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6381:
URL: https://github.com/apache/ozone/pull/6381#issuecomment-1999058059

   <img width="857" alt="Screenshot 2024-03-15 at 12 06 18 AM" src="https://github.com/apache/ozone/assets/2691807/b108f600-979a-4279-b53b-077a14bdfae6">
   


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang closed pull request #6381: HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase
URL: https://github.com/apache/ozone/pull/6381


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6381:
URL: https://github.com/apache/ozone/pull/6381#issuecomment-1999050818

   After this change, isVolumeFull() CPU time reduced from >2% to less than 0.2%


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #6381:
URL: https://github.com/apache/ozone/pull/6381#issuecomment-2215434251

   Closing this one in favor of #6383 . Can port that to HDDS-7593 if needed. Thx!


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl closed pull request #6381: HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase
URL: https://github.com/apache/ozone/pull/6381


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on code in PR #6381:
URL: https://github.com/apache/ozone/pull/6381#discussion_r1533749522


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -138,6 +144,18 @@ public HddsDispatcher(ConfigurationSource config, ContainerSet contSet,
             LOG,
             HddsUtils::processForDebug,
             HddsUtils::processForDebug);
+    this.cachedVolumeUsage = CacheBuilder.newBuilder()
+        .maximumSize(1000)
+        .refreshAfterWrite(1, TimeUnit.MINUTES)

Review Comment:
   I agree with @kerneltime here.
   
   Would reducing this to 1 second see much performance impact?



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #6381:
URL: https://github.com/apache/ozone/pull/6381#discussion_r1534281951


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -138,6 +144,18 @@ public HddsDispatcher(ConfigurationSource config, ContainerSet contSet,
             LOG,
             HddsUtils::processForDebug,
             HddsUtils::processForDebug);
+    this.cachedVolumeUsage = CacheBuilder.newBuilder()
+        .maximumSize(1000)
+        .refreshAfterWrite(1, TimeUnit.MINUTES)

Review Comment:
   1 second should be fine. Need an expiration to prevent it from aggressively refreshing volume stats unnecessarily.



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #6381:
URL: https://github.com/apache/ozone/pull/6381#discussion_r1605190007


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java:
##########
@@ -133,6 +133,46 @@ public void setReserved(long reserved) {
     this.reservedInBytes = reserved;
   }
 
+  /**
+   * Convenience class to calculate minimum free space.
+   */
+  public static class MinFreeSpaceCalculator {
+    private final boolean minFreeSpaceConfigured;
+    private final boolean minFreeSpacePercentConfigured;
+    private final long freeSpace;
+    private float freeSpacePercent;
+    public MinFreeSpaceCalculator(ConfigurationSource conf) {
+      minFreeSpaceConfigured = conf.isConfigured(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE);
+      minFreeSpacePercentConfigured = conf.isConfigured(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT);
+      freeSpace = (long)conf.getStorageSize(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE,
+          HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_DEFAULT, StorageUnit.BYTES);
+      if (minFreeSpacePercentConfigured) {
+        freeSpacePercent = Float.parseFloat(
+            conf.get(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT));
+      }
+    }
+
+    public long get(long capacity) {

Review Comment:
   added



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on code in PR #6381:
URL: https://github.com/apache/ozone/pull/6381#discussion_r1605191128


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -138,6 +144,18 @@ public HddsDispatcher(ConfigurationSource config, ContainerSet contSet,
             LOG,
             HddsUtils::processForDebug,
             HddsUtils::processForDebug);
+    this.cachedVolumeUsage = CacheBuilder.newBuilder()
+        .maximumSize(1000)
+        .refreshAfterWrite(1, TimeUnit.MINUTES)

Review Comment:
   made it update 1 second periodically until 1 minute after the last access.



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #6381:
URL: https://github.com/apache/ozone/pull/6381#issuecomment-1998907985

   Ok looks like this is not enough. I need to cache more info to avoid overhead. Close it for now. Will reopen.


-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #6381:
URL: https://github.com/apache/ozone/pull/6381#discussion_r1532601591


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/volume/VolumeUsage.java:
##########
@@ -133,6 +133,46 @@ public void setReserved(long reserved) {
     this.reservedInBytes = reserved;
   }
 
+  /**
+   * Convenience class to calculate minimum free space.
+   */
+  public static class MinFreeSpaceCalculator {
+    private final boolean minFreeSpaceConfigured;
+    private final boolean minFreeSpacePercentConfigured;
+    private final long freeSpace;
+    private float freeSpacePercent;
+    public MinFreeSpaceCalculator(ConfigurationSource conf) {
+      minFreeSpaceConfigured = conf.isConfigured(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE);
+      minFreeSpacePercentConfigured = conf.isConfigured(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT);
+      freeSpace = (long)conf.getStorageSize(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE,
+          HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_DEFAULT, StorageUnit.BYTES);
+      if (minFreeSpacePercentConfigured) {
+        freeSpacePercent = Float.parseFloat(
+            conf.get(HDDS_DATANODE_VOLUME_MIN_FREE_SPACE_PERCENT));
+      }
+    }
+
+    public long get(long capacity) {

Review Comment:
   Minor nit: It is useful to have some sanity tests for these calculations. 



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


Re: [PR] HDDS-9842. [hsync] Checking disk capacity at every write request is expensive for HBase [ozone]

Posted by "kerneltime (via GitHub)" <gi...@apache.org>.
kerneltime commented on code in PR #6381:
URL: https://github.com/apache/ozone/pull/6381#discussion_r1532602887


##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java:
##########
@@ -138,6 +144,18 @@ public HddsDispatcher(ConfigurationSource config, ContainerSet contSet,
             LOG,
             HddsUtils::processForDebug,
             HddsUtils::processForDebug);
+    this.cachedVolumeUsage = CacheBuilder.newBuilder()
+        .maximumSize(1000)
+        .refreshAfterWrite(1, TimeUnit.MINUTES)

Review Comment:
   1 minute is probably too aggressive? 



-- 
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: issues-unsubscribe@ozone.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org