From df623ebfe3af98ddc592c4cddb6da7a38b2a6975 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Sun, 5 Mar 2017 05:14:20 -0800 Subject: [PATCH] Fix a couple bugs due to calling Period.getMillis(). (#4006) --- .../server/lookup/PollingLookupFactory.java | 7 +++-- .../metadata/SQLMetadataRuleManager.java | 26 +++++++++++-------- .../SQLMetadataRuleManagerProvider.java | 12 ++++----- .../metadata/SQLMetadataRuleManagerTest.java | 4 +-- 4 files changed, 27 insertions(+), 22 deletions(-) diff --git a/extensions-core/lookups-cached-single/src/main/java/io/druid/server/lookup/PollingLookupFactory.java b/extensions-core/lookups-cached-single/src/main/java/io/druid/server/lookup/PollingLookupFactory.java index 3766120ff79..2333ea48d16 100644 --- a/extensions-core/lookups-cached-single/src/main/java/io/druid/server/lookup/PollingLookupFactory.java +++ b/extensions-core/lookups-cached-single/src/main/java/io/druid/server/lookup/PollingLookupFactory.java @@ -23,7 +23,6 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.annotation.JsonTypeName; import com.google.common.annotations.VisibleForTesting; - import io.druid.java.util.common.logger.Logger; import io.druid.query.lookup.LookupExtractorFactory; import io.druid.query.lookup.LookupIntrospectHandler; @@ -63,7 +62,11 @@ public class PollingLookupFactory implements LookupExtractorFactory this.pollPeriod = pollPeriod == null ? Period.ZERO : pollPeriod; this.dataFetcher = dataFetcher; this.cacheFactory = cacheFactory; - this.pollingLookup = new PollingLookup(this.pollPeriod.getMillis(), this.dataFetcher, this.cacheFactory); + this.pollingLookup = new PollingLookup( + this.pollPeriod.toStandardDuration().getMillis(), + this.dataFetcher, + this.cacheFactory + ); } @VisibleForTesting diff --git a/server/src/main/java/io/druid/metadata/SQLMetadataRuleManager.java b/server/src/main/java/io/druid/metadata/SQLMetadataRuleManager.java index b6f60fd7944..a9605e7fec1 100644 --- a/server/src/main/java/io/druid/metadata/SQLMetadataRuleManager.java +++ b/server/src/main/java/io/druid/metadata/SQLMetadataRuleManager.java @@ -22,7 +22,7 @@ package io.druid.metadata; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Supplier; +import com.google.common.base.Preconditions; import com.google.common.base.Throwables; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; @@ -133,8 +133,8 @@ public class SQLMetadataRuleManager implements MetadataRuleManager private static final EmittingLogger log = new EmittingLogger(SQLMetadataRuleManager.class); private final ObjectMapper jsonMapper; - private final Supplier config; - private final Supplier dbTables; + private final MetadataRuleManagerConfig config; + private final MetadataStorageTablesConfig dbTables; private final IDBI dbi; private final AtomicReference>> rules; private final AuditManager auditManager; @@ -151,8 +151,8 @@ public class SQLMetadataRuleManager implements MetadataRuleManager @Inject public SQLMetadataRuleManager( @Json ObjectMapper jsonMapper, - Supplier config, - Supplier dbTables, + MetadataRuleManagerConfig config, + MetadataStorageTablesConfig dbTables, SQLMetadataConnector connector, AuditManager auditManager ) @@ -163,6 +163,10 @@ public class SQLMetadataRuleManager implements MetadataRuleManager this.dbi = connector.getDBI(); this.auditManager = auditManager; + // Verify configured Periods can be treated as Durations (fail-fast before they're needed). + Preconditions.checkNotNull(config.getAlertThreshold().toStandardDuration()); + Preconditions.checkNotNull(config.getPollDuration().toStandardDuration()); + this.rules = new AtomicReference<>( ImmutableMap.>of() ); @@ -178,7 +182,7 @@ public class SQLMetadataRuleManager implements MetadataRuleManager exec = MoreExecutors.listeningDecorator(Execs.scheduledSingleThreaded("DatabaseRuleManager-Exec--%d")); - createDefaultRule(dbi, getRulesTable(), config.get().getDefaultRule(), jsonMapper); + createDefaultRule(dbi, getRulesTable(), config.getDefaultRule(), jsonMapper); future = exec.scheduleWithFixedDelay( new Runnable() { @@ -194,7 +198,7 @@ public class SQLMetadataRuleManager implements MetadataRuleManager } }, 0, - config.get().getPollDuration().toStandardDuration().getMillis(), + config.getPollDuration().toStandardDuration().getMillis(), TimeUnit.MILLISECONDS ); @@ -300,7 +304,7 @@ public class SQLMetadataRuleManager implements MetadataRuleManager retryStartTime = System.currentTimeMillis(); } - if (System.currentTimeMillis() - retryStartTime > config.get().getAlertThreshold().getMillis()) { + if (System.currentTimeMillis() - retryStartTime > config.getAlertThreshold().toStandardDuration().getMillis()) { log.makeAlert(e, "Exception while polling for rules") .emit(); retryStartTime = 0; @@ -328,8 +332,8 @@ public class SQLMetadataRuleManager implements MetadataRuleManager if (theRules.get(dataSource) != null) { retVal.addAll(theRules.get(dataSource)); } - if (theRules.get(config.get().getDefaultRule()) != null) { - retVal.addAll(theRules.get(config.get().getDefaultRule())); + if (theRules.get(config.getDefaultRule()) != null) { + retVal.addAll(theRules.get(config.getDefaultRule())); } return retVal; } @@ -398,6 +402,6 @@ public class SQLMetadataRuleManager implements MetadataRuleManager private String getRulesTable() { - return dbTables.get().getRulesTable(); + return dbTables.getRulesTable(); } } diff --git a/server/src/main/java/io/druid/metadata/SQLMetadataRuleManagerProvider.java b/server/src/main/java/io/druid/metadata/SQLMetadataRuleManagerProvider.java index 26e7dfd77f9..5c885340a44 100644 --- a/server/src/main/java/io/druid/metadata/SQLMetadataRuleManagerProvider.java +++ b/server/src/main/java/io/druid/metadata/SQLMetadataRuleManagerProvider.java @@ -20,10 +20,8 @@ package io.druid.metadata; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.base.Supplier; import com.google.common.base.Throwables; import com.google.inject.Inject; - import io.druid.audit.AuditManager; import io.druid.java.util.common.lifecycle.Lifecycle; import io.druid.server.audit.SQLAuditManager; @@ -34,8 +32,8 @@ import org.skife.jdbi.v2.IDBI; public class SQLMetadataRuleManagerProvider implements MetadataRuleManagerProvider { private final ObjectMapper jsonMapper; - private final Supplier config; - private final Supplier dbTables; + private final MetadataRuleManagerConfig config; + private final MetadataStorageTablesConfig dbTables; private final SQLMetadataConnector connector; private final Lifecycle lifecycle; private final IDBI dbi; @@ -44,8 +42,8 @@ public class SQLMetadataRuleManagerProvider implements MetadataRuleManagerProvid @Inject public SQLMetadataRuleManagerProvider( ObjectMapper jsonMapper, - Supplier config, - Supplier dbTables, + MetadataRuleManagerConfig config, + MetadataStorageTablesConfig dbTables, SQLMetadataConnector connector, Lifecycle lifecycle, SQLAuditManager auditManager @@ -72,7 +70,7 @@ public class SQLMetadataRuleManagerProvider implements MetadataRuleManagerProvid { connector.createRulesTable(); SQLMetadataRuleManager.createDefaultRule( - dbi, dbTables.get().getRulesTable(), config.get().getDefaultRule(), jsonMapper + dbi, dbTables.getRulesTable(), config.getDefaultRule(), jsonMapper ); } diff --git a/server/src/test/java/io/druid/metadata/SQLMetadataRuleManagerTest.java b/server/src/test/java/io/druid/metadata/SQLMetadataRuleManagerTest.java index ba449ef4c80..4d2eb5eff06 100644 --- a/server/src/test/java/io/druid/metadata/SQLMetadataRuleManagerTest.java +++ b/server/src/test/java/io/druid/metadata/SQLMetadataRuleManagerTest.java @@ -74,8 +74,8 @@ public class SQLMetadataRuleManagerTest connector.createRulesTable(); ruleManager = new SQLMetadataRuleManager( mapper, - Suppliers.ofInstance(new MetadataRuleManagerConfig()), - Suppliers.ofInstance(tablesConfig), + new MetadataRuleManagerConfig(), + tablesConfig, connector, auditManager );