belugabehr commented on a change in pull request #1703:
URL: https://github.com/apache/hive/pull/1703#discussion_r531715773
##########
File path:
hcatalog/server-extensions/src/main/java/org/apache/hive/hcatalog/listener/DbNotificationListener.java
##########
@@ -1242,64 +1244,50 @@ private void process(NotificationEvent event,
ListenerEvent listenerEvent) throw
}
private static class CleanerThread extends Thread {
- private RawStore rs;
+ private final RawStore rs;
private int ttl;
- private boolean shouldRun = true;
private long sleepTime;
CleanerThread(Configuration conf, RawStore rs) {
super("DB-Notification-Cleaner");
- this.rs = rs;
- boolean isReplEnabled = MetastoreConf.getBoolVar(conf,
ConfVars.REPLCMENABLED);
- if(isReplEnabled){
- setTimeToLive(MetastoreConf.getTimeVar(conf,
ConfVars.REPL_EVENT_DB_LISTENER_TTL,
- TimeUnit.SECONDS));
- }
- else {
- setTimeToLive(MetastoreConf.getTimeVar(conf,
ConfVars.EVENT_DB_LISTENER_TTL,
- TimeUnit.SECONDS));
- }
- setCleanupInterval(MetastoreConf.getTimeVar(conf,
ConfVars.EVENT_DB_LISTENER_CLEAN_INTERVAL,
- TimeUnit.MILLISECONDS));
setDaemon(true);
+ this.rs = Objects.requireNonNull(rs);
+
+ boolean isReplEnabled = MetastoreConf.getBoolVar(conf,
ConfVars.REPLCMENABLED);
+ ConfVars ttlConf = (isReplEnabled) ?
ConfVars.REPL_EVENT_DB_LISTENER_TTL : ConfVars.EVENT_DB_LISTENER_TTL;
+ setTimeToLive(MetastoreConf.getTimeVar(conf, ttlConf, TimeUnit.SECONDS));
+ setCleanupInterval(
+ MetastoreConf.getTimeVar(conf,
ConfVars.EVENT_DB_LISTENER_CLEAN_INTERVAL, TimeUnit.MILLISECONDS));
}
@Override
public void run() {
- while (shouldRun) {
+ while (true) {
+ LOG.debug("Cleaner thread running");
try {
rs.cleanNotificationEvents(ttl);
rs.cleanWriteNotificationEvents(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: ", ex);
+ LOG.warn("Exception received while cleaning notifications", ex);
Review comment:
Hey, fair question, and I considered that, however,
`InterruptedException` is a checked-exception and neither of these two methods
declare it so it will never be thrown here (unless the signature changes later
to throw one). In fact, these methods do not throw any checked exceptions at
all.
```
void cleanWriteNotificationEvents(int olderThan);
void cleanNotificationEvents(int olderThan);
```
So, if the thread is interrupted at any point, it will eventually make its
ways to the `Thread.sleep()` call and throw the `InterruptedException` at that
time (and exit).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
