Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
d9563f2
Add support for concurrent batch Append and Replace
AmatyaAvadhanula Jun 12, 2023
35cc335
Merge remote-tracking branch 'upstream/master' into overlordSimulator
AmatyaAvadhanula Aug 1, 2023
4d01445
Fix compile errors
AmatyaAvadhanula Aug 2, 2023
5acf93b
Fix compilation in test
AmatyaAvadhanula Aug 2, 2023
be23936
Clean stray comments and move test tasks under test
AmatyaAvadhanula Aug 3, 2023
6a9e6e7
Address preliminary feedback
AmatyaAvadhanula Aug 7, 2023
e3e9cf3
Fix version logic
AmatyaAvadhanula Aug 7, 2023
2ed61fd
Fix checkstyle
AmatyaAvadhanula Aug 8, 2023
0d9b5e6
Merge remote-tracking branch 'upstream/master' into overlordSimulator
AmatyaAvadhanula Aug 16, 2023
38a0071
Fix test
AmatyaAvadhanula Aug 16, 2023
f5144a0
Handle lock type when not specified in context
AmatyaAvadhanula Aug 17, 2023
1413a30
Use Intervals.utc
AmatyaAvadhanula Aug 18, 2023
468e4a2
Fix merge conflicts
AmatyaAvadhanula Aug 18, 2023
d495c3c
Modify segment commit sql statement
AmatyaAvadhanula Aug 21, 2023
671c01c
Fix merge conflicts
AmatyaAvadhanula Aug 21, 2023
311d0ca
Clean up append and replace actions
kfaraz Aug 21, 2023
5e4876b
Merge branch 'overlordSimulator' of github.com:AmatyaAvadhanula/druid…
kfaraz Aug 21, 2023
a2732ca
Fix SQL, remove forbidden APIs
kfaraz Aug 21, 2023
8f1e165
Add unit tests
AmatyaAvadhanula Aug 22, 2023
83d9484
Resolve merge conflicts
AmatyaAvadhanula Aug 22, 2023
06cf8d3
Add comments
AmatyaAvadhanula Aug 22, 2023
5981130
Fix intellij inspections
AmatyaAvadhanula Aug 23, 2023
073bc26
Remove unnecessary exception handling
AmatyaAvadhanula Aug 23, 2023
12534ff
Merge remote-tracking branch 'upstream/master' into overlordSimulator
AmatyaAvadhanula Aug 23, 2023
b271f1f
Fix failing tests
AmatyaAvadhanula Aug 23, 2023
725265c
Add new test task class
AmatyaAvadhanula Aug 23, 2023
41c3cbe
A replacing task must look only at its own replace locks during commit
AmatyaAvadhanula Aug 23, 2023
1fabacf
Merge remote-tracking branch 'upstream/master' into overlordSimulator
AmatyaAvadhanula Aug 24, 2023
5963be3
Remove some unused fields
kfaraz Aug 25, 2023
9fd156c
Merge branch 'overlordSimulator' of github.com:AmatyaAvadhanula/druid…
kfaraz Aug 27, 2023
71023ca
Add some javadocs
kfaraz Aug 29, 2023
a1c22a8
Get tests running
kfaraz Sep 3, 2023
7b0e259
Fix IndexerStorageCoordinatorTest
kfaraz Sep 3, 2023
258caed
Merge branch 'master' of github.com:apache/druid into overlordSimulator
kfaraz Sep 4, 2023
2fd2b9e
Fix for upstream changes
kfaraz Sep 4, 2023
55eca90
Use supervisorTaskId inside ReplaceTaskLock
kfaraz Sep 4, 2023
1ba0e8d
Add CommandQueueTask, remove unneeded classes
kfaraz Sep 6, 2023
ff71674
Fix synchronization in CommandQueueTask
kfaraz Sep 6, 2023
4e0587f
Move some tests
kfaraz Sep 7, 2023
359a923
Add more tests
kfaraz Sep 11, 2023
2bb3b79
Add tests for multiple granularities
kfaraz Sep 19, 2023
ab0b400
Remove extra change
kfaraz Sep 19, 2023
8bb5a13
Fix tests
kfaraz Sep 20, 2023
ae5e7c4
Merge branch 'master' of github.com:apache/druid into overlordSimulator
kfaraz Sep 20, 2023
9c7d5b2
Remove unused dependency
kfaraz Sep 20, 2023
17ab844
Add more tests
kfaraz Sep 20, 2023
e2b04d4
Use correct init version for APPEND locks
kfaraz Sep 21, 2023
37640b7
Merge branch 'master' of github.com:apache/druid into overlordSimulator
kfaraz Sep 21, 2023
5f5c5bf
Fix IndexerSQLCoordinator tests
kfaraz Sep 22, 2023
444cfe4
Add tests for new TaskLocks utility methods
kfaraz Sep 23, 2023
6309702
Add tests for ReplaceTaskLock and SqlSegMetaManagerProvider
kfaraz Sep 23, 2023
f5b7092
Merge branch 'master' of github.com:apache/druid into overlordSimulator
kfaraz Sep 23, 2023
58433d0
Add CreateTablesTest, rename new table to upgradeSegments
kfaraz Sep 24, 2023
a88da61
Cleanup
kfaraz Sep 24, 2023
057252e
Fix spotted bugs
kfaraz Sep 24, 2023
7df51f2
Fix checkstyle
kfaraz Sep 24, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public MetadataStorageTablesConfig getMetadataStorageTablesConfig()
null,
null,
null,
null,
segmentTable,
null,
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@
import java.util.Set;

/**
* Insert segments into metadata storage. The segment versions must all be less than or equal to a lock held by
* your task for the segment intervals.
* <p/>
* Word of warning: Very large "segments" sets can cause oversized audit log entries, which is bad because it means
* that the task cannot actually complete. Callers should avoid this by avoiding inserting too many segments in the
* same action.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.druid.indexing.common.actions;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.type.TypeReference;
import org.apache.druid.indexing.common.task.IndexTaskUtils;
import org.apache.druid.indexing.common.task.Task;
import org.apache.druid.indexing.overlord.CriticalAction;
import org.apache.druid.indexing.overlord.SegmentPublishResult;
import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
import org.apache.druid.metadata.ReplaceTaskLock;
import org.apache.druid.segment.SegmentUtils;
import org.apache.druid.timeline.DataSegment;

import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Append segments to metadata storage. The segment versions must all be less than or equal to a lock held by
* your task for the segment intervals.
Comment thread
AmatyaAvadhanula marked this conversation as resolved.
*/
public class SegmentTransactionalAppendAction implements TaskAction<SegmentPublishResult>
{
private final Set<DataSegment> segments;

public static SegmentTransactionalAppendAction create(Set<DataSegment> segments)
{
return new SegmentTransactionalAppendAction(segments);
}

@JsonCreator
private SegmentTransactionalAppendAction(
@JsonProperty("segments") Set<DataSegment> segments
)
{
this.segments = segments;
}

@JsonProperty
public Set<DataSegment> getSegments()
{
return segments;
}

@Override
public TypeReference<SegmentPublishResult> getReturnTypeReference()
{
return new TypeReference<SegmentPublishResult>()
{
};
}

/**
* Performs some sanity checks and publishes the given segments.
*/
@Override
public SegmentPublishResult perform(Task task, TaskActionToolbox toolbox)
{
TaskLocks.checkLockCoversSegments(task, toolbox.getTaskLockbox(), segments);

final String datasource = task.getDataSource();
final Map<DataSegment, ReplaceTaskLock> segmentToReplaceLock
= TaskLocks.findReplaceLocksCoveringSegments(datasource, toolbox.getTaskLockbox(), segments);

final SegmentPublishResult retVal;
try {
retVal = toolbox.getTaskLockbox().doInCriticalSection(
task,
segments.stream().map(DataSegment::getInterval).collect(Collectors.toSet()),
CriticalAction.<SegmentPublishResult>builder()
.onValidLocks(
() -> toolbox.getIndexerMetadataStorageCoordinator().commitAppendSegments(
segments,
segmentToReplaceLock
)
)
.onInvalidLocks(
() -> SegmentPublishResult.fail(
"Invalid task locks. Maybe they are revoked by a higher priority task."
+ " Please check the overlord log for details."
)
Comment on lines +98 to +101
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wish we could get a better error message than this... Ah well

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should have logged the intervals though.

)
.build()
);
}
catch (Exception e) {
throw new RuntimeException(e);
}

// Emit metrics
final ServiceMetricEvent.Builder metricBuilder = new ServiceMetricEvent.Builder();
IndexTaskUtils.setTaskDimensions(metricBuilder, task);

if (retVal.isSuccess()) {
toolbox.getEmitter().emit(metricBuilder.setMetric("segment/txn/success", 1));
for (DataSegment segment : retVal.getSegments()) {
IndexTaskUtils.setSegmentDimensions(metricBuilder, segment);
toolbox.getEmitter().emit(metricBuilder.setMetric("segment/added/bytes", segment.getSize()));
}
} else {
toolbox.getEmitter().emit(metricBuilder.setMetric("segment/txn/failure", 1));
}

return retVal;
}

@Override
public boolean isAudited()
{
return true;
}

@Override
public String toString()
{
return "SegmentTransactionalAppendAction{" +
"segments=" + SegmentUtils.commaSeparatedIdentifiers(segments) +
'}';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

package org.apache.druid.indexing.common.actions;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.fasterxml.jackson.core.type.TypeReference;
import com.google.common.collect.ImmutableSet;
import org.apache.druid.indexing.common.task.IndexTaskUtils;
import org.apache.druid.indexing.common.task.Task;
import org.apache.druid.indexing.overlord.CriticalAction;
import org.apache.druid.indexing.overlord.SegmentPublishResult;
import org.apache.druid.java.util.emitter.service.ServiceMetricEvent;
import org.apache.druid.metadata.ReplaceTaskLock;
import org.apache.druid.query.DruidMetrics;
import org.apache.druid.segment.SegmentUtils;
import org.apache.druid.timeline.DataSegment;

import java.util.Set;
import java.util.stream.Collectors;

/**
* Replace segments in metadata storage. The segment versions must all be less than or equal to a lock held by
* your task for the segment intervals.
Comment on lines +40 to +41
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this javadoc is not very clear. versions of "what" segments? The ones being replaced? Also what does it mean here by "your" task. Some verbosity here could be helpful.

*/
public class SegmentTransactionalReplaceAction implements TaskAction<SegmentPublishResult>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There's a bunch of stuff that I see and commented on in SegmentTransactionalAppendAction already that seems to exist here too. I'm guessing it's copy pasta. Please clean this class up similar to how you clean up the other one.

{
/**
* Set of segments to be inserted into metadata storage
*/
private final Set<DataSegment> segments;

public static SegmentTransactionalReplaceAction create(
Set<DataSegment> segmentsToPublish
)
{
return new SegmentTransactionalReplaceAction(segmentsToPublish);
}

@JsonCreator
private SegmentTransactionalReplaceAction(
@JsonProperty("segments") Set<DataSegment> segments
)
{
this.segments = ImmutableSet.copyOf(segments);
}

@JsonProperty
public Set<DataSegment> getSegments()
{
return segments;
}

@Override
public TypeReference<SegmentPublishResult> getReturnTypeReference()
{
return new TypeReference<SegmentPublishResult>()
{
};
}

/**
* Performs some sanity checks and publishes the given segments.
*/
@Override
public SegmentPublishResult perform(Task task, TaskActionToolbox toolbox)
{
TaskLocks.checkLockCoversSegments(task, toolbox.getTaskLockbox(), segments);

// Find the active replace locks held only by this task
final Set<ReplaceTaskLock> replaceLocksForTask
= toolbox.getTaskLockbox().findReplaceLocksForTask(task);

final SegmentPublishResult retVal;
try {
retVal = toolbox.getTaskLockbox().doInCriticalSection(
task,
segments.stream().map(DataSegment::getInterval).collect(Collectors.toSet()),
CriticalAction.<SegmentPublishResult>builder()
.onValidLocks(
() -> toolbox.getIndexerMetadataStorageCoordinator()
.commitReplaceSegments(segments, replaceLocksForTask)
)
.onInvalidLocks(
() -> SegmentPublishResult.fail(
"Invalid task locks. Maybe they are revoked by a higher priority task."
+ " Please check the overlord log for details."
)
)
.build()
);
}
catch (Exception e) {
throw new RuntimeException(e);
}

// Emit metrics
final ServiceMetricEvent.Builder metricBuilder = new ServiceMetricEvent.Builder();
IndexTaskUtils.setTaskDimensions(metricBuilder, task);

if (retVal.isSuccess()) {
toolbox.getEmitter().emit(metricBuilder.setMetric("segment/txn/success", 1));

for (DataSegment segment : retVal.getSegments()) {
final String partitionType = segment.getShardSpec() == null ? null : segment.getShardSpec().getType();
metricBuilder.setDimension(DruidMetrics.PARTITIONING_TYPE, partitionType);
metricBuilder.setDimension(DruidMetrics.INTERVAL, segment.getInterval().toString());
Comment thread
abhishekagarwal87 marked this conversation as resolved.
toolbox.getEmitter().emit(metricBuilder.setMetric("segment/added/bytes", segment.getSize()));
}
} else {
toolbox.getEmitter().emit(metricBuilder.setMetric("segment/txn/failure", 1));
}

return retVal;
}

@Override
public boolean isAudited()
{
return true;
}

@Override
public String toString()
{
return "SegmentTransactionalReplaceAction{" +
"segments=" + SegmentUtils.commaSeparatedIdentifiers(segments) +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@
@JsonSubTypes.Type(name = "lockRelease", value = LockReleaseAction.class),
@JsonSubTypes.Type(name = "segmentInsertion", value = SegmentInsertAction.class),
@JsonSubTypes.Type(name = "segmentTransactionalInsert", value = SegmentTransactionalInsertAction.class),
@JsonSubTypes.Type(name = "segmentTransactionalAppend", value = SegmentTransactionalAppendAction.class),
@JsonSubTypes.Type(name = "segmentTransactionalReplace", value = SegmentTransactionalReplaceAction.class),
// Type name doesn't correspond to the name of the class for backward compatibility.
@JsonSubTypes.Type(name = "segmentListUsed", value = RetrieveUsedSegmentsAction.class),
// Type name doesn't correspond to the name of the class for backward compatibility.
Expand Down
Loading