Copilot commented on code in PR #6783:
URL: https://github.com/apache/hive/pull/6783#discussion_r4019104981
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java:
##########
@@ -691,49 +697,58 @@ private static Collection<String>
catalogsToCache(RawStore rs) throws MetaExcept
* @param conf
* @param runOnlyOnce
* @param shouldRunPrewarm
- */ static synchronized void startCacheUpdateService(Configuration conf,
boolean runOnlyOnce,
+ */ static void startCacheUpdateService(Configuration conf, boolean
runOnlyOnce,
boolean shouldRunPrewarm) {
- if (cacheUpdateMaster == null) {
- initBlackListWhiteList(conf);
- if (!MetastoreConf.getBoolVar(conf, ConfVars.HIVE_IN_TEST)) {
- cacheRefreshPeriodMS =
- MetastoreConf.getTimeVar(conf,
ConfVars.CACHED_RAW_STORE_CACHE_UPDATE_FREQUENCY, TimeUnit.MILLISECONDS);
- }
- LOG.info("CachedStore: starting cache update service (run every {} ms)",
cacheRefreshPeriodMS);
- cacheUpdateMaster = Executors.newScheduledThreadPool(1, new
ThreadFactory() {
- @Override public Thread newThread(Runnable r) {
- Thread t = Executors.defaultThreadFactory().newThread(r);
- t.setName("CachedStore-CacheUpdateService: Thread-" + t.getId());
- t.setDaemon(true);
- return t;
+ // Fast path: after first initialization this is a per-worker-thread no-op
(setConf
+ // calls it on every RawStore construction), so skip the lock entirely.
+ if (cacheUpdateMaster != null && !runOnlyOnce) {
+ return;
+ }
+ synchronized (CACHE_UPDATE_SERVICE_LOCK) {
+ if (cacheUpdateMaster == null) {
+ initBlackListWhiteList(conf);
+ if (!MetastoreConf.getBoolVar(conf, ConfVars.HIVE_IN_TEST)) {
+ cacheRefreshPeriodMS =
+ MetastoreConf.getTimeVar(conf,
ConfVars.CACHED_RAW_STORE_CACHE_UPDATE_FREQUENCY, TimeUnit.MILLISECONDS);
+ }
+ LOG.info("CachedStore: starting cache update service (run every {}
ms)", cacheRefreshPeriodMS);
+ cacheUpdateMaster = Executors.newScheduledThreadPool(1, new
ThreadFactory() {
+ @Override public Thread newThread(Runnable r) {
+ Thread t = Executors.defaultThreadFactory().newThread(r);
+ t.setName("CachedStore-CacheUpdateService: Thread-" + t.getId());
+ t.setDaemon(true);
+ return t;
+ }
+ });
+ if (!runOnlyOnce) {
+ cacheUpdateMaster
+ .scheduleAtFixedRate(new CacheUpdateMasterWork(conf,
shouldRunPrewarm), 0, cacheRefreshPeriodMS,
+ TimeUnit.MILLISECONDS);
}
- });
- if (!runOnlyOnce) {
- cacheUpdateMaster
- .scheduleAtFixedRate(new CacheUpdateMasterWork(conf,
shouldRunPrewarm), 0, cacheRefreshPeriodMS,
- TimeUnit.MILLISECONDS);
}
- }
- if (runOnlyOnce) {
- // Some tests control the execution of the background update thread
- cacheUpdateMaster.schedule(new CacheUpdateMasterWork(conf,
shouldRunPrewarm), 0, TimeUnit.MILLISECONDS);
+ if (runOnlyOnce) {
+ // Some tests control the execution of the background update thread
+ cacheUpdateMaster.schedule(new CacheUpdateMasterWork(conf,
shouldRunPrewarm), 0, TimeUnit.MILLISECONDS);
+ }
}
}
- @VisibleForTesting static synchronized boolean stopCacheUpdateService(long
timeout) {
- boolean tasksStoppedBeforeShutdown = false;
- if (cacheUpdateMaster != null) {
- LOG.info("CachedStore: shutting down cache update service");
- try {
- tasksStoppedBeforeShutdown =
cacheUpdateMaster.awaitTermination(timeout, TimeUnit.MILLISECONDS);
- } catch (InterruptedException e) {
- LOG.info("CachedStore: cache update service was interrupted while
waiting for tasks to "
- + "complete before shutting down. Will make a hard stop now.");
+ @VisibleForTesting static boolean stopCacheUpdateService(long timeout) {
+ synchronized (CACHE_UPDATE_SERVICE_LOCK) {
+ boolean tasksStoppedBeforeShutdown = false;
Review Comment:
This lock no longer serializes shutdown with `triggerPreWarm`, so
`stopCacheUpdateService` can call `shutdownNow()` and clear `cacheUpdateMaster`
while the prewarm task still owns `CachedStore.class` and continues executing.
`prewarm()` catches interruption and keeps going, while existing callers
reset/reuse `sharedCache` immediately after this method, allowing concurrent
cache mutation (and a later start can create a second service). Keep shutdown
serialized with prewarm or add explicit cancellation/completion coordination
before clearing the executor.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
