Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,9 @@ These Coordinator static configurations can be defined in the `coordinator/runti
|`druid.coordinator.kill.audit.on`| Boolean value for whether to enable automatic deletion of audit logs. If set to true, Coordinator will periodically remove audit logs from the audit table entries in metadata storage.| No | False|
|`druid.coordinator.kill.audit.period`| How often to do automatic deletion of audit logs in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) duration format. Value must be greater than `druid.coordinator.period.metadataStoreManagementPeriod`. Only applies if `druid.coordinator.kill.audit.on` is set to True.| No| `P1D`|
|`druid.coordinator.kill.audit.durationToRetain`| Duration of audit logs to be retained from created time in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) duration format. Only applies if `druid.coordinator.kill.audit.on` is set to True.| Yes if `druid.coordinator.kill.audit.on` is set to True| None|
|`druid.coordinator.kill.rule.on`| Boolean value for whether to enable automatic deletion of rules. If set to true, Coordinator will periodically remove rules of inactive datasource (datasource with no used and unused segments) from the rule table in metadata storage.| No | False|
|`druid.coordinator.kill.rule.period`| How often to do automatic deletion of rules in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) duration format. Value must be greater than `druid.coordinator.period.metadataStoreManagementPeriod`. Only applies if `druid.coordinator.kill.rule.on` is set to True.| No| `P1D`|
|`druid.coordinator.kill.rule.durationToRetain`| Duration of rules to be retained from created time in [ISO 8601](https://en.wikipedia.org/wiki/ISO_8601) duration format. Only applies if `druid.coordinator.kill.rule.on` is set to True.| Yes if `druid.coordinator.kill.rule.on` is set to True| None|
Copy link
Copy Markdown
Contributor

@suneet-s suneet-s May 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should update this to say that this will only work as expected if the version is a timestamp in utc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add in a followup PR


##### Segment Management
|Property|Possible Values|Description|Default|
Expand Down
1 change: 1 addition & 0 deletions docs/operations/metrics.md
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ These metrics are for the Druid Coordinator and are reset each time the Coordina
|`coordinator/time`|Approximate Coordinator duty runtime in milliseconds. The duty dimension is the string alias of the Duty that is being run.|duty.|Varies.|
|`coordinator/global/time`|Approximate runtime of a full coordination cycle in milliseconds. The `dutyGroup` dimension indicates what type of coordination this run was. i.e. Historical Management vs Indexing|`dutyGroup`|Varies.|
|`metadata/kill/audit/count`|Total number of audit logs automatically deleted from metadata store audit table per each Coordinator kill audit duty run. This metric can help adjust `druid.coordinator.kill.audit.durationToRetain` configuration based on if more or less audit logs need to be deleted per cycle. Note that this metric is only emitted when `druid.coordinator.kill.audit.on` is set to true.| |Varies.|
|`metadata/kill/rule/count`|Total number of rules automatically deleted from metadata store rule table per each Coordinator kill rule duty run. This metric can help adjust `druid.coordinator.kill.rule.durationToRetain` configuration based on if more or less rules need to be deleted per cycle. Note that this metric is only emitted when `druid.coordinator.kill.rule.on` is set to true.| |Varies.|


If `emitBalancingStats` is set to `true` in the Coordinator [dynamic configuration](../configuration/index.md#dynamic-configuration), then [log entries](../configuration/logging.md) for class
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,12 @@ public interface MetadataRuleManager
List<Rule> getRulesWithDefault(String dataSource);

boolean overrideRule(String dataSource, List<Rule> rulesConfig, AuditInfo auditInfo);

/**
* Remove rules for non-existence datasource (datasource with no segment) created older than the given timestamp.
*
* @param timestamp timestamp in milliseconds
* @return number of rules removed
*/
int removeRulesForEmptyDatasourcesOlderThan(long timestamp);
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.skife.jdbi.v2.StatementContext;
import org.skife.jdbi.v2.TransactionCallback;
import org.skife.jdbi.v2.TransactionStatus;
import org.skife.jdbi.v2.Update;
import org.skife.jdbi.v2.tweak.HandleCallback;
import org.skife.jdbi.v2.tweak.ResultSetMapper;

Expand Down Expand Up @@ -389,6 +390,8 @@ public Void inTransaction(Handle handle, TransactionStatus transactionStatus) th
.build(),
handle
);
// Note that the method removeRulesForEmptyDatasourcesOlderThan depends on the version field
// to be a timestamp
String version = auditTime.toString();
handle.createStatement(
StringUtils.format(
Expand Down Expand Up @@ -421,8 +424,40 @@ public Void inTransaction(Handle handle, TransactionStatus transactionStatus) th
return true;
}

@Override
public int removeRulesForEmptyDatasourcesOlderThan(long timestamp)
{
// Note that this DELETE SQL depends on the version field to be a timestamp. Hence, this
// method depends on overrideRule method to set version to timestamp when the rule entry is created
DateTime dateTime = DateTimes.utc(timestamp);
synchronized (lock) {
return dbi.withHandle(
handle -> {
Update sql = handle.createStatement(
// Note that this query could be expensive when the segments table is large
// However, since currently this query is run very infrequent (by default once a day by the KillRules Coordinator duty)
// and the inner query on segment table is a READ (no locking), it is keep this way.
StringUtils.format(
"DELETE FROM %1$s WHERE datasource NOT IN (SELECT DISTINCT datasource from %2$s) and datasource!=:default_rule and version < :date_time",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After this change, the version must be the timestamp of when the rule entry was added. It means this code must remain the same as it is unless we change this query together. Please add comments on both codes so that we won't forget it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, this query could be expensive when the segments table is large. I think maybe it's better to implement the join in this class. For example, this method can iterate all datasources in the rules table and check whether there is any entry in the segments table for each datasource. Then, you can use a much cheaper query for the existence check such as SELECT datasource FROM segments where datasource = ${rule_datasource} LIMIT 1. Maybe this approach needs to add a new index on datasource for the segments table.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comment on version as suggested.

Copy link
Copy Markdown
Contributor Author

@maytasm maytasm Apr 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the query "DELETE FROM %1$s WHERE datasource NOT IN (SELECT DISTINCT datasource from %2$s) and datasource!=:default_rule and version < :date_time", I think since it’s run very infrequent (by default once a day) and the inner query on segment table is a READ (no locking), it should be ok. Also, adding an index on datasource column in the segment table (for the alternative approach suggested) puts additional overhead on the database operation which is probably not worth it for a query that is run once a day. I think it is fine to keep it as is. I also added a comment in the code regarding this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense 👍

getRulesTable(),
getSegmentsTable()
)
);
return sql.bind("default_rule", config.getDefaultRule())
.bind("date_time", dateTime.toString())
.execute();
}
);
}
}

private String getRulesTable()
{
return dbTables.getRulesTable();
}

private String getSegmentsTable()
{
return dbTables.getSegmentsTable();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,14 @@ public abstract class DruidCoordinatorConfig
@Default("PT-1s")
public abstract Duration getCoordinatorAuditKillDurationToRetain();

@Config("druid.coordinator.kill.rule.period")
@Default("P1D")
public abstract Duration getCoordinatorRuleKillPeriod();

@Config("druid.coordinator.kill.rule.durationToRetain")
@Default("PT-1s")
public abstract Duration getCoordinatorRuleKillDurationToRetain();

@Config("druid.coordinator.load.timeout")
public Duration getLoadTimeoutDelay()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.druid.server.coordinator.duty;

import com.google.common.base.Preconditions;
import com.google.inject.Inject;
import org.apache.druid.java.util.common.logger.Logger;
import org.apache.druid.java.util.emitter.service.ServiceEmitter;
import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
import org.apache.druid.server.coordinator.DruidCoordinatorConfig;
import org.apache.druid.server.coordinator.DruidCoordinatorRuntimeParams;

public class KillRules implements CoordinatorDuty
{
private static final Logger log = new Logger(KillRules.class);

private final long period;
private final long retainDuration;
private long lastKillTime = 0;

@Inject
public KillRules(
DruidCoordinatorConfig config
)
{
this.period = config.getCoordinatorRuleKillPeriod().getMillis();
Preconditions.checkArgument(
this.period >= config.getCoordinatorMetadataStoreManagementPeriod().getMillis(),
"coordinator rule kill period must be >= druid.coordinator.period.metadataStoreManagementPeriod"
);
this.retainDuration = config.getCoordinatorRuleKillDurationToRetain().getMillis();
Preconditions.checkArgument(this.retainDuration >= 0, "coordinator rule kill retainDuration must be >= 0");
log.debug(
"Rule Kill Task scheduling enabled with period [%s], retainDuration [%s]",
this.period,
this.retainDuration
);
}

@Override
public DruidCoordinatorRuntimeParams run(DruidCoordinatorRuntimeParams params)
{
if ((lastKillTime + period) < System.currentTimeMillis()) {
lastKillTime = System.currentTimeMillis();

long timestamp = System.currentTimeMillis() - retainDuration;
int ruleRemoved = params.getDatabaseRuleManager().removeRulesForEmptyDatasourcesOlderThan(timestamp);
ServiceEmitter emitter = params.getEmitter();
emitter.emit(
new ServiceMetricEvent.Builder().build(
"metadata/kill/rule/count",
ruleRemoved
)
);
log.info("Finished running KillRules duty. Removed %,d rule", ruleRemoved);
}
return params;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,24 @@
import com.fasterxml.jackson.core.type.TypeReference;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import org.apache.druid.audit.AuditEntry;
import org.apache.druid.audit.AuditInfo;
import org.apache.druid.audit.AuditManager;
import org.apache.druid.client.DruidServer;
import org.apache.druid.jackson.DefaultObjectMapper;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.Intervals;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.segment.TestHelper;
import org.apache.druid.server.audit.SQLAuditManager;
import org.apache.druid.server.audit.SQLAuditManagerConfig;
import org.apache.druid.server.coordinator.rules.IntervalLoadRule;
import org.apache.druid.server.coordinator.rules.Rule;
import org.apache.druid.server.metrics.NoopServiceEmitter;
import org.apache.druid.timeline.DataSegment;
import org.apache.druid.timeline.partition.NoneShardSpec;
import org.junit.After;
import org.junit.Assert;
import org.junit.Before;
Expand All @@ -55,8 +60,9 @@ public class SQLMetadataRuleManagerTest
private MetadataStorageTablesConfig tablesConfig;
private SQLMetadataRuleManager ruleManager;
private AuditManager auditManager;
private SQLMetadataSegmentPublisher publisher;
private final ObjectMapper mapper = new DefaultObjectMapper();

private final ObjectMapper jsonMapper = TestHelper.makeJsonMapper();

@Before
public void setUp()
Expand All @@ -80,6 +86,12 @@ public void setUp()
connector,
auditManager
);
connector.createSegmentTable();
publisher = new SQLMetadataSegmentPublisher(
jsonMapper,
derbyConnectorRule.metadataTablesConfigSupplier().get(),
connector
);
}

@Test
Expand Down Expand Up @@ -201,6 +213,143 @@ public void testFetchAuditEntriesForAllDataSources() throws Exception
}
}

@Test
public void testRemoveRulesOlderThanWithNonExistenceDatasourceAndOlderThanTimestampShouldDelete()
{
List<Rule> rules = ImmutableList.of(
new IntervalLoadRule(
Intervals.of("2015-01-01/2015-02-01"), ImmutableMap.of(
DruidServer.DEFAULT_TIER,
DruidServer.DEFAULT_NUM_REPLICANTS
)
)
);
AuditInfo auditInfo = new AuditInfo("test_author", "test_comment", "127.0.0.1");
ruleManager.overrideRule(
"test_dataSource",
rules,
auditInfo
);
// Verify that rule was added
ruleManager.poll();
Map<String, List<Rule>> allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("test_dataSource").size());

// Now delete rules
ruleManager.removeRulesForEmptyDatasourcesOlderThan(System.currentTimeMillis());

// Verify that rule was deleted
ruleManager.poll();
allRules = ruleManager.getAllRules();
Assert.assertEquals(0, allRules.size());
}

@Test
public void testRemoveRulesOlderThanWithNonExistenceDatasourceAndNewerThanTimestampShouldNotDelete()
{
List<Rule> rules = ImmutableList.of(
new IntervalLoadRule(
Intervals.of("2015-01-01/2015-02-01"), ImmutableMap.of(
DruidServer.DEFAULT_TIER,
DruidServer.DEFAULT_NUM_REPLICANTS
)
)
);
AuditInfo auditInfo = new AuditInfo("test_author", "test_comment", "127.0.0.1");
ruleManager.overrideRule(
"test_dataSource",
rules,
auditInfo
);
// Verify that rule was added
ruleManager.poll();
Map<String, List<Rule>> allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("test_dataSource").size());

// This will not delete the rule as the rule was created just now so it will have the created timestamp later than
// the timestamp 2012-01-01T00:00:00Z
ruleManager.removeRulesForEmptyDatasourcesOlderThan(DateTimes.of("2012-01-01T00:00:00Z").getMillis());

// Verify that rule was not deleted
ruleManager.poll();
allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("test_dataSource").size());
}

@Test
public void testRemoveRulesOlderThanWithActiveDatasourceShouldNotDelete() throws Exception
{
List<Rule> rules = ImmutableList.of(
new IntervalLoadRule(
Intervals.of("2015-01-01/2015-02-01"), ImmutableMap.of(
DruidServer.DEFAULT_TIER,
DruidServer.DEFAULT_NUM_REPLICANTS
)
)
);
AuditInfo auditInfo = new AuditInfo("test_author", "test_comment", "127.0.0.1");
ruleManager.overrideRule(
"test_dataSource",
rules,
auditInfo
);

// Verify that rule was added
ruleManager.poll();
Map<String, List<Rule>> allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("test_dataSource").size());

// Add segment metadata to segment table so that the datasource is considered active
DataSegment dataSegment = new DataSegment(
"test_dataSource",
Intervals.of("2015-01-01/2015-02-01"),
"1",
ImmutableMap.of(
"type", "s3_zip",
"bucket", "test",
"key", "test_dataSource/xxx"
),
ImmutableList.of("dim1", "dim2", "dim3"),
ImmutableList.of("count", "value"),
NoneShardSpec.instance(),
1,
1234L
);
publisher.publishSegment(dataSegment);

// This will not delete the rule as the datasource has segment in the segment metadata table
ruleManager.removeRulesForEmptyDatasourcesOlderThan(System.currentTimeMillis());

// Verify that rule was not deleted
ruleManager.poll();
allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("test_dataSource").size());
}

@Test
public void testRemoveRulesOlderThanShouldNotDeleteDefault()
{
// Create the default rule
ruleManager.start();
// Verify the default rule
ruleManager.poll();
Map<String, List<Rule>> allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("_default").size());
// Delete everything
ruleManager.removeRulesForEmptyDatasourcesOlderThan(System.currentTimeMillis());
// Verify the default rule was not deleted
ruleManager.poll();
allRules = ruleManager.getAllRules();
Assert.assertEquals(1, allRules.size());
Assert.assertEquals(1, allRules.get("_default").size());
}

@After
public void cleanup()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ public void setUp() throws Exception
null,
null,
null,
null,
null,
10,
new Duration("PT0s")
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ public void setUp() throws Exception
null,
null,
null,
null,
null,
10,
new Duration("PT0s")
);
Expand Down
Loading