Skip to content

Commit

Permalink
Fix a couple bugs due to calling Period.getMillis(). (apache#4006)
Browse files Browse the repository at this point in the history
  • Loading branch information
gianm authored and nishantmonu51 committed Mar 5, 2017
1 parent 337f387 commit df623eb
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
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;
Expand Down Expand Up @@ -63,7 +62,11 @@ public PollingLookupFactory(
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
Expand Down
26 changes: 15 additions & 11 deletions server/src/main/java/io/druid/metadata/SQLMetadataRuleManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
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;
Expand Down Expand Up @@ -133,8 +133,8 @@ public Void withHandle(Handle handle) throws Exception
private static final EmittingLogger log = new EmittingLogger(SQLMetadataRuleManager.class);

private final ObjectMapper jsonMapper;
private final Supplier<MetadataRuleManagerConfig> config;
private final Supplier<MetadataStorageTablesConfig> dbTables;
private final MetadataRuleManagerConfig config;
private final MetadataStorageTablesConfig dbTables;
private final IDBI dbi;
private final AtomicReference<ImmutableMap<String, List<Rule>>> rules;
private final AuditManager auditManager;
Expand All @@ -151,8 +151,8 @@ public Void withHandle(Handle handle) throws Exception
@Inject
public SQLMetadataRuleManager(
@Json ObjectMapper jsonMapper,
Supplier<MetadataRuleManagerConfig> config,
Supplier<MetadataStorageTablesConfig> dbTables,
MetadataRuleManagerConfig config,
MetadataStorageTablesConfig dbTables,
SQLMetadataConnector connector,
AuditManager auditManager
)
Expand All @@ -163,6 +163,10 @@ public SQLMetadataRuleManager(
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.<String, List<Rule>>of()
);
Expand All @@ -178,7 +182,7 @@ public void start()

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()
{
Expand All @@ -194,7 +198,7 @@ public void run()
}
},
0,
config.get().getPollDuration().toStandardDuration().getMillis(),
config.getPollDuration().toStandardDuration().getMillis(),
TimeUnit.MILLISECONDS
);

Expand Down Expand Up @@ -300,7 +304,7 @@ public Map<String, List<Rule>> fold(
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;
Expand Down Expand Up @@ -328,8 +332,8 @@ public List<Rule> getRulesWithDefault(final String dataSource)
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;
}
Expand Down Expand Up @@ -398,6 +402,6 @@ public Void inTransaction(Handle handle, TransactionStatus transactionStatus) th

private String getRulesTable()
{
return dbTables.get().getRulesTable();
return dbTables.getRulesTable();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,8 +32,8 @@
public class SQLMetadataRuleManagerProvider implements MetadataRuleManagerProvider
{
private final ObjectMapper jsonMapper;
private final Supplier<MetadataRuleManagerConfig> config;
private final Supplier<MetadataStorageTablesConfig> dbTables;
private final MetadataRuleManagerConfig config;
private final MetadataStorageTablesConfig dbTables;
private final SQLMetadataConnector connector;
private final Lifecycle lifecycle;
private final IDBI dbi;
Expand All @@ -44,8 +42,8 @@ public class SQLMetadataRuleManagerProvider implements MetadataRuleManagerProvid
@Inject
public SQLMetadataRuleManagerProvider(
ObjectMapper jsonMapper,
Supplier<MetadataRuleManagerConfig> config,
Supplier<MetadataStorageTablesConfig> dbTables,
MetadataRuleManagerConfig config,
MetadataStorageTablesConfig dbTables,
SQLMetadataConnector connector,
Lifecycle lifecycle,
SQLAuditManager auditManager
Expand All @@ -72,7 +70,7 @@ public void start() throws Exception
{
connector.createRulesTable();
SQLMetadataRuleManager.createDefaultRule(
dbi, dbTables.get().getRulesTable(), config.get().getDefaultRule(), jsonMapper
dbi, dbTables.getRulesTable(), config.getDefaultRule(), jsonMapper
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ public void setUp()
connector.createRulesTable();
ruleManager = new SQLMetadataRuleManager(
mapper,
Suppliers.ofInstance(new MetadataRuleManagerConfig()),
Suppliers.ofInstance(tablesConfig),
new MetadataRuleManagerConfig(),
tablesConfig,
connector,
auditManager
);
Expand Down

0 comments on commit df623eb

Please sign in to comment.