kabhishek4 commented on code in PR #5605:
URL: https://github.com/apache/hbase/pull/5605#discussion_r1445999734
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/PrefetchExecutor.java:
##########
@@ -134,4 +139,25 @@ private PrefetchExecutor() {
static ScheduledExecutorService getExecutorPool() {
return prefetchExecutorPool;
}
+
+ public static int getPrefetchDelay() {
+ return prefetchDelayMillis;
+ }
+
+ @Override
+ public void onConfigurationChange(Configuration conf) {
+ PrefetchExecutor.loadConfiguration(conf);
+ }
+
+ @Override
+ public void registerChildren(ConfigurationManager manager) {
+ }
+
+ @Override
+ public void deregisterChildren(ConfigurationManager manager) {
+ }
+
+ public static void loadConfiguration(Configuration conf) {
+ prefetchDelayMillis = conf.getInt(PREFETCH_DELAY, 1000);
Review Comment:
Yes, in the scenario you described, despite the delay is changed, prefetched
threads will wait. There are 2 ways to address this in my opinion.
1. Keep the behavior as it is and document above mentioned limitation
2. Interrupt threads which are doing prefetch, update the new value and
restart the prefetch threads
In approach 2, there are some complexities as HFilePreadReader class
'drives' the prefetch. Also, run() of HFilePreadReader class provides 'work
context' to the prefetch threads. Now, onConfigurationChange needs to cancel
the prefetch threads and restart them. For this can we store 'work context' in
a map
for ex,
private static final Map<Path, Runnable> prefetchRunnable = new
ConcurrentSkipListMap<>();
and populate during request() method so that we can restart the prefetch
threads in following way,
For ex,
@Override
public void onConfigurationChange(Configuration conf) {
PrefetchExecutor.loadConfiguration(conf);
}
public static void loadConfiguration(Configuration conf) {
prefetchDelayMillis = conf.getInt(PREFETCH_DELAY, 1000);
prefetchFutures.forEach((k, v) -> {
cancel(k);
request(k,prefetchRunnable.get(k));
LOG.debug("Reset called on Prefetch of file {}", k);
});
}
Please comment. Also, let me know if there is another alternative.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/PrefetchExecutor.java:
##########
@@ -134,4 +139,25 @@ private PrefetchExecutor() {
static ScheduledExecutorService getExecutorPool() {
return prefetchExecutorPool;
}
+
+ public static int getPrefetchDelay() {
+ return prefetchDelayMillis;
+ }
+
+ @Override
+ public void onConfigurationChange(Configuration conf) {
+ PrefetchExecutor.loadConfiguration(conf);
+ }
+
+ @Override
+ public void registerChildren(ConfigurationManager manager) {
+ }
+
+ @Override
+ public void deregisterChildren(ConfigurationManager manager) {
+ }
+
+ public static void loadConfiguration(Configuration conf) {
+ prefetchDelayMillis = conf.getInt(PREFETCH_DELAY, 1000);
Review Comment:
This document has been attached to the JIRA HBASE-28292.
--
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]
