HBASE-27997 Enhance prefetch executor to record region prefetch infor… #5339
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…mation along with the list of hfiles prefetched
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
…mation along with the list of hfiles prefetched
|
🎊 +1 overall
This message was automatically generated. |
wchevreuil
reviewed
Aug 2, 2023
| prefetchCompleted.get(hFileName).entrySet().iterator().next(); | ||
| String regionEncodedName = regionEntry.getKey(); | ||
| long filePrefetchedSize = regionEntry.getValue(); | ||
| if (LOG.isDebugEnabled()) { |
There was a problem hiding this comment.
Nit: no need for this check, since we are not doing any computation on the debug message.
| @@ -53,7 +56,8 @@ public final class PrefetchExecutor { | |||
| private static final Map<Path, Future<?>> 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<String, Boolean> prefetchCompleted = new HashMap<>(); | |||
| private static ConcurrentHashMap<String, Long> regionPrefetchSizeMap = new ConcurrentHashMap<>(); | |||
| private static HashMap<String, Map<String, Long>> prefetchCompleted = new HashMap<>(); | |||
There was a problem hiding this comment.
Should use Map<String,Pair<String,Long>> prefetchCompleted.
There was a problem hiding this comment.
Updated the code as per the suggestion. Replaced HashMap and ConcurrentHashMap to Map.
There was a problem hiding this comment.
How about using Pair<String,Long> as the map value? We don't really need a map as the value here. You can use org.apache.hadoop.hbase.util.Pair here, I believe it would be clearer to understand the structure.
There was a problem hiding this comment.
Good catch Wellington, Thanks for pointing it out. Made the change in the updated patch.
| @@ -53,7 +56,8 @@ public final class PrefetchExecutor { | |||
| private static final Map<Path, Future<?>> 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<String, Boolean> prefetchCompleted = new HashMap<>(); | |||
| private static ConcurrentHashMap<String, Long> regionPrefetchSizeMap = new ConcurrentHashMap<>(); | |||
There was a problem hiding this comment.
OOP good practices: "Program to interfaces"Map<String, Long> regionPrefetchSizeMap = new ConcurrentHashMap<>();
…mation along with the list of hfiles prefetched
…mation along with the list of hfiles prefetched
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
wchevreuil
approved these changes
Aug 2, 2023
|
The UT failures are falkey and unrelated to this PR changers. |
wchevreuil
pushed a commit
to wchevreuil/hbase
that referenced
this pull request
Aug 23, 2023
apache#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com>
wchevreuil
pushed a commit
that referenced
this pull request
Nov 10, 2023
#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com>
wchevreuil
pushed a commit
that referenced
this pull request
Nov 13, 2023
#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com>
wchevreuil
pushed a commit
that referenced
this pull request
Nov 14, 2023
#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com>
wchevreuil
pushed a commit
that referenced
this pull request
Nov 15, 2023
#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com>
wchevreuil
pushed a commit
to wchevreuil/hbase
that referenced
this pull request
Nov 22, 2023
apache#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com>
ragarkar
added a commit
to ragarkar/hbase
that referenced
this pull request
Nov 27, 2023
apache#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com> (cherry picked from commit 364fcea)
ragarkar
added a commit
to ragarkar/hbase
that referenced
this pull request
Nov 28, 2023
apache#5339) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewew-by: Kota-SH <shanmukhaharipriya@gmail.com> (cherry picked from commit 364fcea)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…mation along with the list of hfiles prefetched