Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add versions to markUsed and markUnused APIs #16141

Merged

Conversation

abhishekrb19
Copy link
Contributor

Follow up to #15994, this patch adds support for versions to the markUsed and markUnused APIs.

Segments can now be marked as used or unused within the specified interval using an optional list of versions, i.e., (interval, [versions]). When versions is unspecified, all versions of segments in the interval are marked as used or unused, preserving the old behavior before this patch.

Release note

Segments can now be marked as used or unused within the specified interval using an optional list of versions, i.e., (interval, [versions]). When versions is unspecified, all versions of segments in the interval are marked as used or unused, preserving the old behavior.

Sample markUsed API with versions:

curl 'http://localhost:8888/druid/coordinator/v1/datasources/foo/markUsed' \
  -H 'Accept: application/json, text/plain, */*' \
  -H 'Content-Type: application/json' \
  -H 'Origin: http://localhost:8888' \
  --data-raw '{"interval":"2020/2030", "versions": ["2024-03-14T13:30:54.460Z"]}' 

{"numChangedSegments":15}

Sample markUnused API with versions:

curl 'http://localhost:8888/druid/coordinator/v1/datasources/foo/markUnused' \
  -H 'Accept: application/json, text/plain, */*' \
  -H 'Content-Type: application/json' \
  -H 'Origin: http://localhost:8888' \
  --data-raw '{"interval":"2020/2030", "versions": ["2024-03-14T16:00:04.086Z"]}' 

{"numChangedSegments":15}

Key changed/added classes in this PR
  • SegmentsMetadataManager.java
  • SqlSegmentsMetadataManager.java
  • SqlSegmentsMetadataQuery.java
  • DatasourcesResource.java

This PR has:

  • been self-reviewed.
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • been tested in a test Druid cluster.

@abhishekrb19 abhishekrb19 changed the title Mark used and unused APIs by versions. Add versions to markUsed and markUnused APIs Mar 15, 2024
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement, @abhishekrb19 ! I have left some minor comments.

Copy link
Contributor

@zachjsh zachjsh left a comment

Choose a reason for hiding this comment

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

LGTM

@abhishekrb19
Copy link
Contributor Author

Thanks for the reviews, @kfaraz and @zachjsh!
@kfaraz, if you have more comments, I'm happy to address them in a follow-up.

@abhishekrb19 abhishekrb19 merged commit fa8e511 into apache:master Mar 19, 2024
84 checks passed
@abhishekrb19 abhishekrb19 deleted the mark_used_unused_api_by_versions branch March 19, 2024 16:22
Copy link
Contributor

@kfaraz kfaraz left a comment

Choose a reason for hiding this comment

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

@abhishekrb19 , thanks for merging the PR. The implementation made sense to me. I just have a concern with the treatment of null and empty list of versions. Please refer to the comment for the details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants