dengzhhu653 commented on code in PR #6783:
URL: https://github.com/apache/hive/pull/6783#discussion_r4023230368
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java:
##########
@@ -145,27 +155,38 @@ void setConfForTestExceptSharedCache(Configuration conf) {
initBlackListWhiteList(conf);
}
- private static synchronized void triggerUpdateUsingEvent(RawStore rawStore) {
+ private static void triggerUpdateUsingEvent(RawStore rawStore) {
if (!isCachePrewarmed.get()) {
LOG.error("cache update should be done only after prewarm");
throw new RuntimeException("cache update should be done only after
prewarm");
}
- long startTime = System.nanoTime();
- long preEventId = lastEventId;
- try {
- lastEventId = updateUsingNotificationEvents(rawStore, lastEventId);
- } catch (Exception e) {
- LOG.error(" cache update failed for start event id " + lastEventId + "
with error ", e);
- throw new RuntimeException(e.getMessage());
- } finally {
- long endTime = System.nanoTime();
- LOG.info("Time taken in updateUsingNotificationEvents for num events : "
+ (lastEventId - preEventId) + " = "
- + (endTime - startTime) / 1000000 + "ms");
+ synchronized (EVENT_UPDATE_LOCK) {
+ long startTime = System.nanoTime();
+ long preEventId = lastEventId;
+ try {
+ lastEventId = updateUsingNotificationEvents(rawStore, lastEventId);
+ } catch (Exception e) {
+ LOG.error(" cache update failed for start event id " + lastEventId + "
with error ", e);
+ throw new RuntimeException(e.getMessage());
+ } finally {
+ long endTime = System.nanoTime();
+ LOG.info("Time taken in updateUsingNotificationEvents for num events :
" + (lastEventId - preEventId) + " = "
+ + (endTime - startTime) / 1000000 + "ms");
+ }
}
}
- private static synchronized void triggerPreWarm(RawStore rawStore) {
- lastEventId = rawStore.getCurrentNotificationEventId().getEventId();
+ // Deliberately not synchronized (HIVE-30052): the only caller is the first
run of the
+ // single-threaded cacheUpdateMaster executor, and prewarm() is idempotent
via isCachePrewarmed.
+ // Holding a monitor here for the entire prewarm would block every thread
that needs the same
+ // monitor: previously the shared class monitor blocked all RPC worker
threads for the whole
+ // prewarm (in setConf via startCacheUpdateService, and in commitTransaction
via
+ // triggerUpdateUsingEvent when event based updates are enabled).
+ private static void triggerPreWarm(RawStore rawStore) {
+ synchronized (EVENT_UPDATE_LOCK) {
Review Comment:
can we remove this lock?
--
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]
