You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/02/15 21:02:10 UTC
[GitHub] [druid] maytasm opened a new pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
maytasm opened a new pull request #12263:
URL: https://github.com/apache/druid/pull/12263
Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
### Description
User may want to run auto compaction duty more frequently. Auto compaction does already have task slot limits (max slot and ratio) to prevent auto compaction from starving other ingestion tasks. Hence, it is safe to run auto compaction more frequently so that we can consistently utilized all the task slots available to auto compaction. However, before this change auto compaction duty is tied to other indexing duties and changing the period for auto compaction will also impact the other duties. This PR uses https://github.com/apache/druid/pull/11601 functionality that allows coordinator to be dynamically enabled via the configs. With this PR change, user can run auto compaction duty in it's own coordinator group using the following config:
```
druid.coordinator.dutyGroups=["compaction"]
druid.coordinator.compaction.duties=["compactSegments"]
druid.coordinator.compaction.period=PT60S
```
This PR has:
- [x] been self-reviewed.
- [ ] using the [concurrency checklist](https://github.com/apache/druid/blob/master/dev/code-review/concurrency.md) (Remove this item if the PR doesn't have any relation to concurrency.)
- [ ] added documentation for new or modified features or behaviors.
- [x] added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
- [ ] added or updated version, license, or notice information in [licenses.yaml](https://github.com/apache/druid/blob/master/dev/license.md)
- [x] added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
- [x] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
- [ ] added integration tests.
- [x] been tested in a test Druid cluster.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on a change in pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #12263:
URL: https://github.com/apache/druid/pull/12263#discussion_r808568046
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
##########
@@ -797,6 +800,29 @@ private void stopBeingLeader()
return ImmutableList.copyOf(duties);
}
+ private CompactSegments initializeCompactSegmentsDuty()
+ {
+ List<CompactSegments> compactSegmentsDutyFromCustomGroups = getCompactSegmentsDutyFromCustomGroups();
+ if (compactSegmentsDutyFromCustomGroups.isEmpty()) {
+ return new CompactSegments(config, objectMapper, indexingServiceClient);
+ } else {
+ if (compactSegmentsDutyFromCustomGroups.size() > 1) {
+ log.warn("More than one compactSegments duty is configured in the Coordinator Custom Duty Group");
Review comment:
```suggestion
log.warn("More than one compactSegments duty is configured in the Coordinator Custom Duty Group. The first duty will be picked up.");
```
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java
##########
@@ -90,10 +92,11 @@
private final AtomicReference<Map<String, AutoCompactionSnapshot>> autoCompactionSnapshotPerDataSource = new AtomicReference<>();
@Inject
+ @JsonCreator
Review comment:
Please add a serde test for this class.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm merged pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
Posted by GitBox <gi...@apache.org>.
maytasm merged pull request #12263:
URL: https://github.com/apache/druid/pull/12263
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm commented on a change in pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #12263:
URL: https://github.com/apache/druid/pull/12263#discussion_r809505477
##########
File path: docs/design/coordinator.md
##########
@@ -98,6 +98,13 @@ Compaction tasks might fail due to the following reasons.
Once a compaction task fails, the Coordinator simply checks the segments in the interval of the failed task again, and issues another compaction task in the next run.
+Note that Compacting Segments Coordinator Duty is automatically enabled and run as part of the Indexing Service Duties group. However, Compacting Segments Coordinator Duty can be configured to run in isolation as a seperate coordinator duty group. This allow changing the period of Compacting Segments Coordinator Duty without impacting the period of other Indexing Service Duties. This can be done by setting the following properties (for more details see [custom pluggable Coordinator Duty](../development/modules.md#Adding-your-own-custom-pluggable-Coordinator-Duty)):
Review comment:
Done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] jihoonson commented on a change in pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
Posted by GitBox <gi...@apache.org>.
jihoonson commented on a change in pull request #12263:
URL: https://github.com/apache/druid/pull/12263#discussion_r809409304
##########
File path: docs/design/coordinator.md
##########
@@ -98,6 +98,13 @@ Compaction tasks might fail due to the following reasons.
Once a compaction task fails, the Coordinator simply checks the segments in the interval of the failed task again, and issues another compaction task in the next run.
+Note that Compacting Segments Coordinator Duty is automatically enabled and run as part of the Indexing Service Duties group. However, Compacting Segments Coordinator Duty can be configured to run in isolation as a seperate coordinator duty group. This allow changing the period of Compacting Segments Coordinator Duty without impacting the period of other Indexing Service Duties. This can be done by setting the following properties (for more details see [custom pluggable Coordinator Duty](../development/modules.md#Adding-your-own-custom-pluggable-Coordinator-Duty)):
Review comment:
```suggestion
Note that Compacting Segments Coordinator Duty is automatically enabled and run as part of the Indexing Service Duties group. However, Compacting Segments Coordinator Duty can be configured to run in isolation as a seperate coordinator duty group. This allows changing the period of Compacting Segments Coordinator Duty without impacting the period of other Indexing Service Duties. This can be done by setting the following properties (for more details see [custom pluggable Coordinator Duty](../development/modules.md#Adding-your-own-custom-pluggable-Coordinator-Duty)):
```
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm commented on a change in pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #12263:
URL: https://github.com/apache/druid/pull/12263#discussion_r808733841
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/DruidCoordinator.java
##########
@@ -797,6 +800,29 @@ private void stopBeingLeader()
return ImmutableList.copyOf(duties);
}
+ private CompactSegments initializeCompactSegmentsDuty()
+ {
+ List<CompactSegments> compactSegmentsDutyFromCustomGroups = getCompactSegmentsDutyFromCustomGroups();
+ if (compactSegmentsDutyFromCustomGroups.isEmpty()) {
+ return new CompactSegments(config, objectMapper, indexingServiceClient);
+ } else {
+ if (compactSegmentsDutyFromCustomGroups.size() > 1) {
+ log.warn("More than one compactSegments duty is configured in the Coordinator Custom Duty Group");
Review comment:
Done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] maytasm commented on a change in pull request #12263: Allow coordinator run auto compaction duty period to be configured separately from other indexing duties
Posted by GitBox <gi...@apache.org>.
maytasm commented on a change in pull request #12263:
URL: https://github.com/apache/druid/pull/12263#discussion_r808748463
##########
File path: server/src/main/java/org/apache/druid/server/coordinator/duty/CompactSegments.java
##########
@@ -90,10 +92,11 @@
private final AtomicReference<Map<String, AutoCompactionSnapshot>> autoCompactionSnapshotPerDataSource = new AtomicReference<>();
@Inject
+ @JsonCreator
Review comment:
Done
--
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.
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org