[GitHub] [hbase] ragarkar commented on a diff in pull request #5339: HBASE-27997 Enhance prefetch executor to record region prefetch infor…

2023-08-02 Thread via GitHub


ragarkar commented on code in PR #5339:
URL: https://github.com/apache/hbase/pull/5339#discussion_r1281784116


##
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/PrefetchExecutor.java:
##
@@ -53,7 +56,8 @@ public final class PrefetchExecutor {
   private static final Map> prefetchFutures = new 
ConcurrentSkipListMap<>();
   /** Set of files for which prefetch is completed */
   @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = 
"MS_SHOULD_BE_FINAL")
-  private static HashMap prefetchCompleted = new HashMap<>();
+  private static ConcurrentHashMap regionPrefetchSizeMap = new 
ConcurrentHashMap<>();
+  private static HashMap> prefetchCompleted = new 
HashMap<>();

Review Comment:
   Good catch Wellington, Thanks for pointing it out. Made the change in the 
updated patch.



-- 
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-unsubscr...@hbase.apache.org

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



[GitHub] [hbase] ragarkar commented on a diff in pull request #5339: HBASE-27997 Enhance prefetch executor to record region prefetch infor…

2023-08-02 Thread via GitHub


ragarkar commented on code in PR #5339:
URL: https://github.com/apache/hbase/pull/5339#discussion_r1281684251


##
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/PrefetchExecutor.java:
##
@@ -53,7 +56,8 @@ public final class PrefetchExecutor {
   private static final Map> prefetchFutures = new 
ConcurrentSkipListMap<>();
   /** Set of files for which prefetch is completed */
   @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = 
"MS_SHOULD_BE_FINAL")
-  private static HashMap prefetchCompleted = new HashMap<>();
+  private static ConcurrentHashMap regionPrefetchSizeMap = new 
ConcurrentHashMap<>();
+  private static HashMap> prefetchCompleted = new 
HashMap<>();

Review Comment:
   Updated the code as per the suggestion. Replaced HashMap and 
ConcurrentHashMap to Map.



##
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/PrefetchExecutor.java:
##
@@ -53,7 +56,8 @@ public final class PrefetchExecutor {
   private static final Map> prefetchFutures = new 
ConcurrentSkipListMap<>();
   /** Set of files for which prefetch is completed */
   @edu.umd.cs.findbugs.annotations.SuppressWarnings(value = 
"MS_SHOULD_BE_FINAL")
-  private static HashMap prefetchCompleted = new HashMap<>();
+  private static ConcurrentHashMap regionPrefetchSizeMap = new 
ConcurrentHashMap<>();

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.

To unsubscribe, e-mail: issues-unsubscr...@hbase.apache.org

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



[GitHub] [hbase] ragarkar commented on a diff in pull request #5339: HBASE-27997 Enhance prefetch executor to record region prefetch infor…

2023-08-02 Thread via GitHub


ragarkar commented on code in PR #5339:
URL: https://github.com/apache/hbase/pull/5339#discussion_r1281683185


##
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/PrefetchExecutor.java:
##
@@ -120,9 +124,35 @@ public static void request(Path path, Runnable runnable) {
 }
   }
 
-  public static void complete(Path path) {
+  private static void removeFileFromPrefetch(String hFileName) {
+// Update the regionPrefetchedSizeMap before removing the file from 
prefetchCompleted
+if (prefetchCompleted.containsKey(hFileName)) {
+  Map.Entry regionEntry =
+prefetchCompleted.get(hFileName).entrySet().iterator().next();
+  String regionEncodedName = regionEntry.getKey();
+  long filePrefetchedSize = regionEntry.getValue();
+  if (LOG.isDebugEnabled()) {

Review Comment:
   Remove the check.



-- 
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-unsubscr...@hbase.apache.org

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