Skip to content

SegmentLoadDropHandler: Fix deadlock when segments have errors loading on startup. #5735

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.

Sign up for GitHub

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

Merged
b-slim merged 1 commit into from
May 3, 2018
Merged

SegmentLoadDropHandler: Fix deadlock when segments have errors loading on startup. #5735

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 9 additions & 5 deletions server/src/main/java/io/druid/server/coordination/SegmentLoadDropHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,11 @@ public class SegmentLoadDropHandler implements DataSegmentChangeHandler
{
private static final EmittingLogger log = new EmittingLogger(SegmentLoadDropHandler.class);

private final Object lock = new Object();
// Synchronizes removals from segmentsToDelete
private final Object segmentDeleteLock = new Object();

// Synchronizes start/stop of this object.
private final Object startStopLock = new Object();

private final ObjectMapper jsonMapper;
private final SegmentLoaderConfig config;
Expand Down Expand Up @@ -137,7 +141,7 @@ public SegmentLoadDropHandler(
@LifecycleStart
public void start() throws IOException
{
synchronized (lock) {
synchronized (startStopLock) {
if (started) {
return;
}
Expand All @@ -159,7 +163,7 @@ public void start() throws IOException
@LifecycleStop
public void stop()
{
synchronized (lock) {
synchronized (startStopLock) {
if (!started) {
return;
}
Expand Down Expand Up @@ -296,7 +300,7 @@ each time when addSegment() is called, it has to wait for the lock in order to m
things slow. Given that in most cases segmentsToDelete.contains(segment) returns false, it will save a lot of
cost of acquiring lock by doing the "contains" check outside the synchronized block.
*/
synchronized (lock) {
synchronized (segmentDeleteLock) {
segmentsToDelete.remove(segment);
}
}
Expand Down Expand Up @@ -423,7 +427,7 @@ private void removeSegment(
public void run()
{
try {
synchronized (lock) {
synchronized (segmentDeleteLock) {
if (segmentsToDelete.remove(segment)) {
segmentManager.dropSegment(segment);

Expand Down