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
Bulk request which try and fail to create multiple indices may never return #6436
Conversation
…return This is caused by an NPE in the error handling code. All is well if only 1 index fails (or none).
builder.add(client().prepareIndex().setIndex(expectedFailures[i] ? "INVALID.NAME" : "test").setType("type1").setId("1").setSource("field", 1)); | ||
String name; | ||
if (expectedFailures[i]) { | ||
name = twoBadNames[randomIntBetween(0, 1)]; |
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.
you can call randomFrom(twoBadNames)
@@ -624,10 +624,17 @@ public void testThatInvalidIndexNamesShouldNotBreakCompleteBulkRequest() { | |||
int bulkEntryCount = randomIntBetween(10, 50); | |||
BulkRequestBuilder builder = client().prepareBulk(); | |||
boolean[] expectedFailures = new boolean[bulkEntryCount]; | |||
String[] twoBadNames = new String[]{"INVALID.NAME1", "INVALID.NAME2"}; |
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.
since the test didn't catch the bug because there was exactly one bad name, maybe we should also try having more bad names (ie. randomizing the size of this array)?
@@ -624,10 +625,20 @@ public void testThatInvalidIndexNamesShouldNotBreakCompleteBulkRequest() { | |||
int bulkEntryCount = randomIntBetween(10, 50); | |||
BulkRequestBuilder builder = client().prepareBulk(); | |||
boolean[] expectedFailures = new boolean[bulkEntryCount]; | |||
ArrayList<String> badIndexNames = new ArrayList<>(); | |||
for (int i = 1; i < randomIntBetween(1, 5); i++) { |
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 upper bound would be recomputed on each iteration, probably not what you want?
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.
(and starting from i=1 allows for an empty list, is it ok?)
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.
shame on me. fixing.
@jpountz pushed another commit. |
LGTM |
…return This is caused by an NPE in the error handling code. All is well if only 1 index creation fails (or none). Closes #6436
merged. Thx @jpountz |
This is caused by an NPE in the error handling code. All is well if only 1 index fails (or none).