Repository: hive Updated Branches: refs/heads/branch-2 7c6467126 -> 8b3ce4688
HIVE-19050 : DBNotificationListener does not catch exceptions in the cleaner thread (Vihang Karajgaonkar reviewed by Peter Vary) Project: http://git-wip-us.apache.org/repos/asf/hive/repo Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/8b3ce468 Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/8b3ce468 Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/8b3ce468 Branch: refs/heads/branch-2 Commit: 8b3ce4688807ae448794c23fe5094f73205912f3 Parents: 7c646712 Author: Vihang Karajgaonkar <vih...@cloudera.com> Authored: Fri Apr 6 10:02:33 2018 -0700 Committer: Vihang Karajgaonkar <vih...@cloudera.com> Committed: Fri Apr 6 10:18:43 2018 -0700 ---------------------------------------------------------------------- .../listener/DbNotificationListener.java | 19 ++++++++++--- .../listener/DummyRawStoreFailEvent.java | 4 +++ .../listener/TestDbNotificationListener.java | 28 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hive/blob/8b3ce468/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java ---------------------------------------------------------------------- diff --git a/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java b/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java index 0cf51fb..c4e1675 100644 --- a/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java +++ b/hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java @@ -105,7 +105,8 @@ public class DbNotificationListener extends MetaStoreEventListener { private HiveConf hiveConf; private MessageFactory msgFactory; - private synchronized void init(HiveConf conf) throws MetaException { + //cleaner is a static object, use static synchronized to make sure its thread-safe + private static synchronized void init(Configuration conf) throws MetaException { if (cleaner == null) { cleaner = new CleanerThread(conf, RawStoreProxy.getProxy(conf, conf, @@ -120,7 +121,7 @@ public class DbNotificationListener extends MetaStoreEventListener { // with a Configuration parameter, so we have to declare config as Configuration. But it // actually passes a HiveConf, which we need. So we'll do this ugly down cast. hiveConf = (HiveConf)config; - init(hiveConf); + DbNotificationListener.init(conf); msgFactory = MessageFactory.getInstance(); } @@ -512,7 +513,7 @@ public class DbNotificationListener extends MetaStoreEventListener { static private long sleepTime = 60000; CleanerThread(HiveConf conf, RawStore rs) { - super("CleanerThread"); + super("DB-Notification-Cleaner"); this.rs = rs; setTimeToLive(conf.getTimeVar(HiveConf.ConfVars.METASTORE_EVENT_DB_LISTENER_TTL, TimeUnit.SECONDS)); @@ -522,7 +523,17 @@ public class DbNotificationListener extends MetaStoreEventListener { @Override public void run() { while (true) { - rs.cleanNotificationEvents(ttl); + try { + rs.cleanNotificationEvents(ttl); + } catch (Exception ex) { + //catching exceptions here makes sure that the thread doesn't die in case of unexpected + //exceptions + LOG.warn( + "Exception received while cleaning notifications. More details can be found in debug mode" + + ex.getMessage()); + LOG.debug(ex.getMessage(), ex); + } + LOG.debug("Cleaner thread done"); try { Thread.sleep(sleepTime); http://git-wip-us.apache.org/repos/asf/hive/blob/8b3ce468/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java ---------------------------------------------------------------------- diff --git a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java index 9871083..1099480 100644 --- a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java +++ b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/DummyRawStoreFailEvent.java @@ -836,6 +836,10 @@ public class DummyRawStoreFailEvent implements RawStore, Configurable { @Override public void cleanNotificationEvents(int olderThan) { + if (!shouldEventSucceed) { + //throw exception to simulate an issue with cleaner thread + throw new RuntimeException("Dummy exception while cleaning notifications"); + } objectStore.cleanNotificationEvents(olderThan); } http://git-wip-us.apache.org/repos/asf/hive/blob/8b3ce468/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java ---------------------------------------------------------------------- diff --git a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java index 9f406cd..5a40780 100644 --- a/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java +++ b/itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java @@ -1576,4 +1576,32 @@ public class TestDbNotificationListener { LOG.info("second trigger done"); assertEquals(0, rsp2.getEventsSize()); } + + @Test + public void cleanupNotificationWithError() throws Exception { + Database db = new Database("cleanup1", "no description", "file:/tmp", emptyParameters); + msClient.createDatabase(db); + msClient.dropDatabase("cleanup1"); + + LOG.info("Pulling events immediately after createDatabase/dropDatabase"); + NotificationEventResponse rsp = msClient.getNextNotification(firstEventId, 0, null); + assertEquals(2, rsp.getEventsSize()); + //this simulates that cleaning thread will error out while cleaning the notifications + DummyRawStoreFailEvent.setEventSucceed(false); + // sleep for expiry time, and then fetch again + // sleep twice the TTL interval - things should have been cleaned by then. + Thread.sleep(EVENTS_TTL * 2 * 1000); + + LOG.info("Pulling events again after failing to cleanup"); + NotificationEventResponse rsp2 = msClient.getNextNotification(firstEventId, 0, null); + LOG.info("second trigger done"); + assertEquals(2, rsp2.getEventsSize()); + DummyRawStoreFailEvent.setEventSucceed(true); + Thread.sleep(EVENTS_TTL * 2 * 1000); + + LOG.info("Pulling events again after cleanup"); + rsp2 = msClient.getNextNotification(firstEventId, 0, null); + LOG.info("third trigger done"); + assertEquals(0, rsp2.getEventsSize()); + } }
