Honor update request timeout #23825
Merged
Honor update request timeout #23825
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasontedor
added
:Data Management/Indices APIs
When executing an update request, the request timeout is not transferred to the index/delete request executed on behalf of the update request. This leads to update requests not timing out when they should (e.g., if not all shards are available when the request specifies wait_for_shards=all with a small timeout). This commit causes the index/delete requests to honor the update request timeout.
jasontedor
force-pushed
the
update-request-timeout
branch
from
83863b1
to
fc228fc
Compare
nik9000
reviewed
Mar 30, 2017
| @@ -307,12 +338,12 @@ public void testNowInScript() throws IOException { | |||
| Streamable action = result.action(); | |||
| assertThat(action, instanceOf(IndexRequest.class)); | |||
| IndexRequest indexAction = (IndexRequest) action; | |||
| assertEquals(indexAction.sourceAsMap().get("update_timestamp"), nowInMillis); | |||
| assertEquals(nowInMillis, indexAction.sourceAsMap().get("update_timestamp")); | |||
| } | ||
| } | ||
|
|
||
| private Script mockInlineScript(final String script) { |
There was a problem hiding this comment.
If we think this makes things more readable we should probably add it as a static method on MockScriptEngine and statically import it in the tests.
| ESTestCase::randomNonNegativeLong); | ||
| final Streamable action = result.action(); | ||
| if (delete) { | ||
| assertThat(action, instanceOf(DeleteRequest.class)); |
There was a problem hiding this comment.
I tend to prefer letting the test throw the ClassCastException rather than asserting this first.
It should be handled by the index case but I'll add a test to be sure. |
You were right, there was a case missing. I pushed 58b4d8c. |
nik9000
approved these changes
Mar 30, 2017
nik9000
approved these changes
Mar 30, 2017
jasontedor
added a commit
that referenced
this pull request
Mar 30, 2017
When executing an update request, the request timeout is not transferred to the index/delete request executed on behalf of the update request. This leads to update requests not timing out when they should (e.g., if not all shards are available when the request specifies wait_for_shards=all with a small timeout). This commit causes the index/delete requests to honor the update request timeout. Relates #23825
jasontedor
added a commit
that referenced
this pull request
Mar 30, 2017
When executing an update request, the request timeout is not transferred to the index/delete request executed on behalf of the update request. This leads to update requests not timing out when they should (e.g., if not all shards are available when the request specifies wait_for_shards=all with a small timeout). This commit causes the index/delete requests to honor the update request timeout. Relates #23825
|
Thank you @nik9000. |
|
LGTM2 (albeit too late to count) |
Your thoughts are never too late to count, thank you for looking. |
|
Would be doable to push this to 2.x as well? |
|
This is not a critical bug and not a regression, I think that we will not be backporting this one. |
|
+1 to not backporting to 2.4
… On 31 Mar 2017, at 3:22 PM, Jason Tedor ***@***.***> wrote:
This is not a critical bug and not a regression, I think that we will not be backporting this one.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When executing an update request, the request timeout is not transferred to the index/delete request executed on behalf of the update request. This leads to update requests not timing out when they should (e.g., if not all shards are available when the request specifies wait_for_shards=all with a small timeout). This commit causes the index/delete requests to honor the update request timeout.