-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Optimize unused segment query for segment allocation #16623
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3275,4 +3275,49 @@ public void testSegmentIdShouldNotBeReallocated() throws IOException | |||||
| ); | ||||||
| Assert.assertNull(coordinator.retrieveSegmentForId(theId.asSegmentId().toString(), true)); | ||||||
| } | ||||||
|
|
||||||
| @Test | ||||||
| public void testRetrieveUnusedSegmentsForExactIntervalAndVersion() throws Exception | ||||||
| { | ||||||
| DataSegment unusedForDifferentVersion = createSegment( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| Intervals.of("2024/2025"), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: use an interval which is easier to use in a name. You may even assign this interval value to a field named
Suggested change
|
||||||
| "v0", | ||||||
| new NumberedShardSpec(0, 0) | ||||||
| ); | ||||||
| DataSegment unusedSegmentForExactIntervalAndVersion = createSegment( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| Intervals.of("2024/2025"), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| "v1", | ||||||
| new NumberedShardSpec(0, 0) | ||||||
| ); | ||||||
| DataSegment unusedSegmentForDifferentInterval = createSegment( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| Intervals.of("2023/2024"), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rather than a disjoint interval, a better test would be to verify that a segment in an overlapping (but not identical) interval is not returned.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| "v1", | ||||||
| new NumberedShardSpec(0, 0) | ||||||
| ); | ||||||
| coordinator.commitSegments( | ||||||
| ImmutableSet.of( | ||||||
| unusedForDifferentVersion, | ||||||
| unusedSegmentForDifferentInterval, | ||||||
| unusedSegmentForExactIntervalAndVersion | ||||||
| ), | ||||||
| null | ||||||
| ); | ||||||
| coordinator.markSegmentsAsUnusedWithinInterval(DS.WIKI, Intervals.ETERNITY); | ||||||
|
|
||||||
| DataSegment usedSegmentForExactIntervalAndVersion = createSegment( | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| Intervals.of("2024/2025"), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| "v1", | ||||||
| new NumberedShardSpec(1, 0) | ||||||
| ); | ||||||
| coordinator.commitSegments(ImmutableSet.of(usedSegmentForExactIntervalAndVersion), null); | ||||||
|
|
||||||
|
|
||||||
| List<String> unusedSegmentIdsForIntervalAndVersion = | ||||||
| coordinator.retrieveUnusedSegmentIdsForExactIntervalAndVersion(DS.WIKI, Intervals.of("2024/2025"), "v1"); | ||||||
| Assert.assertEquals(1, unusedSegmentIdsForIntervalAndVersion.size()); | ||||||
| Assert.assertEquals( | ||||||
| unusedSegmentForExactIntervalAndVersion.getId().toString(), | ||||||
| unusedSegmentIdsForIntervalAndVersion.get(0) | ||||||
| ); | ||||||
| } | ||||||
| } | ||||||
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.
Nit: