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

Do not allocate week granular segments unless requested #15589

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion examples/quickstart/tutorial/rollup-index.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
],
"granularitySpec" : {
"type" : "uniform",
"segmentGranularity" : "week",
"segmentGranularity" : "day",
"queryGranularity" : "minute",
"intervals" : ["2018-01-01/2018-01-03"],
"rollup" : true
Expand Down
2 changes: 1 addition & 1 deletion examples/quickstart/tutorial/transform-index.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
],
"granularitySpec" : {
"type" : "uniform",
"segmentGranularity" : "week",
"segmentGranularity" : "day",
"queryGranularity" : "minute",
"intervals" : ["2018-01-01/2018-01-03"],
"rollup" : true
Expand Down
2 changes: 1 addition & 1 deletion examples/quickstart/tutorial/updates-append-index.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
],
"granularitySpec": {
"type": "uniform",
"segmentGranularity": "week",
"segmentGranularity": "day",
"queryGranularity": "minute",
"intervals": ["2018-01-01/2018-01-03"],
"rollup": true
Expand Down
2 changes: 1 addition & 1 deletion examples/quickstart/tutorial/updates-append-index2.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
],
"granularitySpec" : {
"type" : "uniform",
"segmentGranularity" : "week",
"segmentGranularity" : "day",
"queryGranularity" : "minute",
"intervals" : ["2018-01-01/2018-01-03"],
"rollup" : true
Expand Down
2 changes: 1 addition & 1 deletion examples/quickstart/tutorial/updates-init-index.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
],
"granularitySpec" : {
"type" : "uniform",
"segmentGranularity" : "week",
"segmentGranularity" : "day",
"queryGranularity" : "minute",
"intervals" : ["2018-01-01/2018-01-03"],
"rollup" : true
Expand Down
2 changes: 1 addition & 1 deletion examples/quickstart/tutorial/updates-overwrite-index.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
],
"granularitySpec" : {
"type" : "uniform",
"segmentGranularity" : "week",
"segmentGranularity" : "day",
"queryGranularity" : "minute",
"intervals" : ["2018-01-01/2018-01-03"],
"rollup" : true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,22 @@
import java.util.stream.Collectors;

/**
* Allocates a pending segment for a given timestamp. The preferredSegmentGranularity is used if there are no prior
* segments for the given timestamp, or if the prior segments for the given timestamp are already at the
* preferredSegmentGranularity. Otherwise, the prior segments will take precedence.
* Allocates a pending segment for a given timestamp.
* If a visible chunk of used segments contains the interval with the query granularity containing the timestamp,
* the pending segment is allocated with its interval.
* Else, if the interval with the preferred segment granularity containing the timestamp has no overlap
* with the existing used segments, the preferred segment granularity is used.
* Else, find the coarsest segment granularity, containing the interval with the query granularity for the timestamp,
* that does not overlap with the existing used segments. This granularity is used for allocation if it exists.
* <p/>
* This action implicitly acquires some task locks when it allocates segments. You do not have to acquire them
* beforehand, although you *do* have to release them yourself. (Note that task locks are automatically released when
* the task is finished.)
* <p/>
* If this action cannot acquire an appropriate task lock, or if it cannot expand an existing segment set, it returns
* null.
* </p>
* Do NOT allocate WEEK granularity segments unless the preferred segment granularity is WEEK.
*/
public class SegmentAllocateAction implements TaskAction<SegmentIdWithShardSpec>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
import org.junit.runners.Parameterized;

import java.io.IOException;
import java.time.Duration;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -995,6 +996,66 @@ public void testAllocateAllGranularity()
Assert.assertEquals(Intervals.ETERNITY, id2.getInterval());
}

@Test
public void testAllocateWeekOnlyWhenWeekIsPreferred()
{
final Task task = NoopTask.create();
taskActionTestKit.getTaskLockbox().add(task);

final SegmentIdWithShardSpec id1 = allocate(
task,
DateTimes.of("2023-12-16"),
Granularities.MINUTE,
Granularities.HOUR,
"s1",
null
);

final SegmentIdWithShardSpec id2 = allocate(
task,
DateTimes.of("2023-12-18"),
Granularities.MINUTE,
Granularities.WEEK,
"s2",
null
);

Assert.assertNotNull(id1);
Assert.assertNotNull(id2);
Assert.assertEquals(Duration.ofHours(1).toMillis(), id1.getInterval().toDurationMillis());
Assert.assertEquals(Duration.ofDays(7).toMillis(), id2.getInterval().toDurationMillis());
}

@Test
public void testAllocateDayWhenMonthNotPossible()
{
final Task task = NoopTask.create();
taskActionTestKit.getTaskLockbox().add(task);

final SegmentIdWithShardSpec id1 = allocate(
task,
DateTimes.of("2023-12-16"),
Granularities.MINUTE,
Granularities.HOUR,
"s1",
null
);

final SegmentIdWithShardSpec id2 = allocate(
task,
DateTimes.of("2023-12-18"),
Granularities.MINUTE,
Granularities.MONTH,
"s2",
null
);

Assert.assertNotNull(id1);
Assert.assertNotNull(id2);
Assert.assertEquals(Duration.ofHours(1).toMillis(), id1.getInterval().toDurationMillis());
Assert.assertEquals(Duration.ofDays(1).toMillis(), id2.getInterval().toDurationMillis());
}

private SegmentIdWithShardSpec allocate(
final Task task,
final DateTime timestamp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ public static Granularity mergeGranularities(List<Granularity> toMerge)
* ALL will not be returned unless the provided granularity is ALL. NONE will never be returned, even if the
* provided granularity is NONE. This is because the main usage of this function in production is segment
* allocation, and we do not wish to generate NONE-granular segments.
*
* The list of granularities returned contains WEEK only if the requested granularity is WEEK.
*/
public static List<Granularity> granularitiesFinerThan(final Granularity gran0)
{
Expand All @@ -117,6 +119,9 @@ public static List<Granularity> granularitiesFinerThan(final Granularity gran0)
if ((gran == GranularityType.ALL && !gran0.equals(Granularities.ALL)) || gran == GranularityType.NONE) {
continue;
}
if (gran == GranularityType.WEEK && !gran0.equals(Granularities.WEEK)) {
continue;
}
final Granularity segmentGranularity = gran.create(origin, tz);
final long segmentGranularityDurationMillis = segmentGranularity.bucket(DateTimes.EPOCH).toDurationMillis();
final long gran0DurationMillis = gran0.bucket(DateTimes.EPOCH).toDurationMillis();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1002,6 +1002,27 @@ public void testGranularitiesFinerThanHour()
);
}

@Test
public void testGranularitiesFinerThanWeek()
{
Assert.assertEquals(
ImmutableList.of(
Granularities.WEEK,
Granularities.DAY,
Granularities.EIGHT_HOUR,
Granularities.SIX_HOUR,
Granularities.HOUR,
Granularities.THIRTY_MINUTE,
Granularities.FIFTEEN_MINUTE,
Granularities.TEN_MINUTE,
Granularities.FIVE_MINUTE,
Granularities.MINUTE,
Granularities.SECOND
),
Granularity.granularitiesFinerThan(Granularities.WEEK)
);
}

@Test
public void testGranularitiesFinerThanAll()
{
Expand All @@ -1011,7 +1032,6 @@ public void testGranularitiesFinerThanAll()
Granularities.YEAR,
Granularities.QUARTER,
Granularities.MONTH,
Granularities.WEEK,
Copy link
Contributor

Choose a reason for hiding this comment

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

For completeness, could we also add a test testGranularitiesFinerThanWeek() that does include the week granularity when it's requested?

Granularities.DAY,
Granularities.EIGHT_HOUR,
Granularities.SIX_HOUR,
Expand Down