-
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
Account for potential gaps in hydrants in sink initialization, hydrant swapping (e.g. h0, h1, h4) #1747
Conversation
|
||
private void testPersistHydrantGapsHelper(final Object commitMetadata) throws Exception | ||
{ | ||
final MutableBoolean committed = new MutableBoolean(false); |
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.
Shouldn't this be an AtomicBoolean? The run() of the committer happens in a different thread.
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.
The other test should probably be changed too
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.
@gianm changed the MutableBooleans to AtomicBooleans
78212ab
to
8be14fe
Compare
@@ -744,6 +744,10 @@ public int compare(File o1, File o2) | |||
) | |||
); | |||
} | |||
if (hydrants.isEmpty()) { | |||
// Probably encountered a corrupt sink directory if no hydrants were loaded, skip sink creation. |
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.
should we log a warning in this case?
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.
@xvrl Added a warning log for persisted sinks with no hydrants
@jon-wei can you describe the scenario in which you see the hydrants going missing? |
👍 |
@xvrl I ran into an issue where a segment had no hydrants at all, and the empty sink that got created caused an exception later on. This could be due to a corrupted segment that didn't get cleaned up completely because I restarted my druid processes during development. The hydrant gap handling was to address a potential issue that @gianm mentioned to me, when there are multiple hydrants being persisted in parallel and don't necessarily complete their persist in order. |
8be14fe
to
066e141
Compare
@@ -744,6 +744,11 @@ public int compare(File o1, File o2) | |||
) | |||
); | |||
} | |||
if (hydrants.isEmpty()) { | |||
// Probably encountered a corrupt sink directory | |||
log.warn("Found persisted sink with no hydrants at %s, skipping sink creation.", sinkDir.getAbsolutePath()); |
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.
Can we instead say. Found persisted segment directory with no intermediate segments present
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.
@fjy adjusted warning message
…t swapping (e.g. h0, h1, h4)
066e141
to
9f6bb03
Compare
👍 will merge after travis |
Account for potential gaps in hydrants in sink initialization, hydrant swapping (e.g. h0, h1, h4)
Fixes issues with sink hydrant handling when loading a persisted sink directory with missing hydrants, e.g. persisted sink directory, with hydrants 1 and 3 missing:
$ pwd
.../test-datasource/1970-01-01T00:00:00.000Z_1970-12-31T00:00:00.000Z
$ ls
0 2 4