-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Add versions to markUsed
and markUnused
APIs
#16141
Conversation
markUsed
and markUnused
APIs
server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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.
server/src/main/java/org/apache/druid/metadata/SegmentsMetadataManager.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/server/http/DataSourcesResource.java
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
Show resolved
Hide resolved
…esource.java Co-authored-by: Kashif Faraz <kashif.faraz@gmail.com>
server/src/main/java/org/apache/druid/metadata/SqlSegmentsMetadataQuery.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/apache/druid/metadata/SqlSegmentsMetadataManagerTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this 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.
Follow up to #15994, this patch adds support for versions to the
markUsed
andmarkUnused
APIs.Segments can now be marked as used or unused within the specified interval using an optional list of versions, i.e.,
(interval, [versions])
. Whenversions
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])
. Whenversions
is unspecified, all versions of segments in the interval are marked as used or unused, preserving the old behavior.Sample
markUsed
API with versions:Sample
markUnused
API with versions:Key changed/added classes in this PR
SegmentsMetadataManager.java
SqlSegmentsMetadataManager.java
SqlSegmentsMetadataQuery.java
DatasourcesResource.java
This PR has: