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 2021/03/16 00:05:46 UTC
[GitHub] [druid] suneet-s opened a new pull request #10996: CompactionTask throws exception on conflicting segmentGranularity
suneet-s opened a new pull request #10996:
URL: https://github.com/apache/druid/pull/10996
### Description
A follow up to #10843
Since there are 2 ways for a user to specify segmentGranularity - this change makes throws an exception if a user accidentally submits a compaction task with conflicting segmentGranularities - by using the old way and the new way at the same time.
TODO: add more details about why this is posisble
<hr>
This PR has:
- [ ] 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.
- [ ] 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)
- [ ] 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.
- [ ] 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.
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 #10996: CompactionTask throws exception on conflicting segmentGranularity
Posted by GitBox <gi...@apache.org>.
maytasm merged pull request #10996:
URL: https://github.com/apache/druid/pull/10996
----------------------------------------------------------------
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:
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] suneet-s commented on pull request #10996: CompactionTask throws exception on conflicting segmentGranularity
Posted by GitBox <gi...@apache.org>.
suneet-s commented on pull request #10996:
URL: https://github.com/apache/druid/pull/10996#issuecomment-800540553
- No integration tests added since this is fully covered by the unit tests and it's a test of edge cases.
- No documentation added because this would conflict with #10935 which is already in flight
----------------------------------------------------------------
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:
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] suneet-s commented on a change in pull request #10996: CompactionTask throws exception on conflicting segmentGranularity
Posted by GitBox <gi...@apache.org>.
suneet-s commented on a change in pull request #10996:
URL: https://github.com/apache/druid/pull/10996#discussion_r594798377
##########
File path: indexing-service/src/main/java/org/apache/druid/indexing/common/task/CompactionTask.java
##########
@@ -207,6 +208,16 @@ public CompactionTask(
this.dimensionsSpec = dimensionsSpec == null ? dimensions : dimensionsSpec;
this.metricsSpec = metricsSpec;
this.segmentGranularity = segmentGranularity;
+ if (granularitySpec != null
+ && segmentGranularity != null
+ && !segmentGranularity.equals(granularitySpec.getSegmentGranularity())) {
Review comment:
I couldn't decide what it means to have a `granularitySpec.getSegmentGranularity()` be null and segmentGranularity be non null, so I figured it would be good to flag that as a conflict as well
----------------------------------------------------------------
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:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org