Avoid setting connection request timeout #30384
Merged
+61
−3
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
We've been setting this value to 500ms in the default low-level REST client configuration, misunderstanding the effect that it would have. This proved very problematic, as it ends up causing `TimeoutException` returned from the leased pool in some cases even for successful requests. Closes elastic#24069
javanna
added
>bug
:Clients/Java Low Level REST Client
|
Pinging @elastic/es-core-infra |
javanna
commented
May 4, 2018
| * Tests sending a bunch of async requests works well (e.g. no TimeoutException from the leased pool) | ||
| * See https://github.com/elastic/elasticsearch/issues/24069 | ||
| */ | ||
| public void testManyAsyncRequests() throws Exception { |
There was a problem hiding this comment.
@nik9000 this test may look familiar to you :)
This was referenced May 4, 2018
|
retest this please |
nik9000
approved these changes
May 8, 2018
docs/CHANGELOG.asciidoc
Outdated
| @@ -199,6 +199,8 @@ coming[6.3.1] | |||
|
|
|||
| Reduce the number of object allocations made by {security} when resolving the indices and aliases for a request ({pull}30180[#30180]) | |||
|
|
|||
| Stop setting connection request timeout in default low-level REST client configuration ({pull}30384[#30384]) | |||
There was a problem hiding this comment.
Maybe add "because it causes timeout exceptions before any requests are made which isn't how timeouts are supposed to work" or something like that.
|
retest this please |
1 similar comment
|
retest this please |
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
May 9, 2018
…or-you * elastic/master: (22 commits) Docs: Test examples that recreate lang analyzers (elastic#29535) BulkProcessor to retry based on status code (elastic#29329) Add GET Repository High Level REST API (elastic#30362) add a comment explaining the need for RetryOnReplicaException on missing mappings Add `coordinating_only` node selector (elastic#30313) Stop forking groovyc (elastic#30471) Avoid setting connection request timeout (elastic#30384) Use date format in `date_range` mapping before fallback to default (elastic#29310) Watcher: Increase HttpClient parallel sent requests (elastic#30130) Mute ML upgrade test (elastic#30458) Stop forking javac (elastic#30462) Client: Deprecate many argument performRequest (elastic#30315) Docs: Use task_id in examples of tasks (elastic#30436) Security: Rename IndexLifecycleManager to SecurityIndexManager (elastic#30442) [Docs] Fix typo in cardinality-aggregation.asciidoc (elastic#30434) Avoid NPE in `more_like_this` when field has zero tokens (elastic#30365) Build: Switch to building javadoc with html5 (elastic#30440) Add a quick tour of the project to CONTRIBUTING (elastic#30187) Reindex: Use request flavored methods (elastic#30317) Silence SplitIndexIT.testSplitIndexPrimaryTerm test failure. (elastic#30432) ...
javanna
added a commit
that referenced
this pull request
May 9, 2018
We've been setting this value to 500ms in the default low-level REST client configuration, misunderstanding the effect that it would have. This proved very problematic, as it ends up causing `TimeoutException` returned from the leased pool in some cases even for successful requests. Closes #24069
dnhatn
added a commit
that referenced
this pull request
May 10, 2018
* master: Upgrade to Lucene-7.4-snapshot-6705632810 (#30519) add version compatibility from 6.4.0 after backport, see #30319 (#30390) Security: Simplify security index listeners (#30466) Add proper longitude validation in geo_polygon_query (#30497) Remove Discovery.AckListener.onTimeout() (#30514) Build: move generated-resources to build (#30366) Reindex: Fold "with all deps" project into reindex (#30154) Isolate REST client single host tests (#30504) Solve Gradle deprecation warnings around shadowJar (#30483) SAML: Process only signed data (#30420) Remove BWC repository test (#30500) Build: Remove xpack specific run task (#30487) AwaitsFix IntegTestZipClientYamlTestSuiteIT#indices.split tests LLClient: Add setJsonEntity (#30447) Expose CommonStatsFlags directly in IndicesStatsRequest. (#30163) Silence IndexUpgradeIT test failures. (#30430) Bump Gradle heap to 1792m (#30484) [docs] add warning for read-write indices in force merge documentation (#28869) Avoid deadlocks in cache (#30461) Test: remove hardcoded list of unconfigured ciphers (#30367) mute SplitIndexIT due to #30416 Docs: Test examples that recreate lang analyzers (#29535) BulkProcessor to retry based on status code (#29329) Add GET Repository High Level REST API (#30362) add a comment explaining the need for RetryOnReplicaException on missing mappings Add `coordinating_only` node selector (#30313) Stop forking groovyc (#30471) Avoid setting connection request timeout (#30384) Use date format in `date_range` mapping before fallback to default (#29310) Watcher: Increase HttpClient parallel sent requests (#30130) # Conflicts: # x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/LocalStateCompositeXPackPlugin.java
dnhatn
added a commit
that referenced
this pull request
May 10, 2018
* 6.x: Upgrade to Lucene-7.4-snapshot-6705632810 (#30519) Remove Discovery.AckListener.onTimeout() (#30514) Build: move generated-resources to build (#30366) Reindex: Fold "with all deps" project into reindex (#30154) Isolate REST client single host tests (#30504) Remove BWC repository test (#30500) Build: Remove xpack specific run task (#30487) AwaitsFix IntegTestZipClientYamlTestSuiteIT#indices.split tests LLClient: Add setJsonEntity (#30447) [docs] add warning for read-write indices in force merge documentation (#28869) Avoid deadlocks in cache (#30461) BulkProcessor to retry based on status code (#29329) Avoid setting connection request timeout (#30384) Test: remove hardcoded list of unconfigured ciphers (#30367) Add GET Repository High Level REST API (#30362) mute SplitIndexIT due to #30416 Docs: Test examples that recreate lang analyzers (#29535) add a comment explaining the need for RetryOnReplicaException on missing mappings Pass the task to broadcast actions (#29672) Stop forking groovyc (#30471) Add `coordinating_only` node selector (#30313) Fix accidental error in changelog Use date format in `date_range` mapping before fallback to default (#29310) Watcher: Increase HttpClient parallel sent requests (#30130) [Security][Tests] Azeri(Turkish) locale tripps opensaml dependency
|
@javanna This may be backported now. |
javanna
added a commit
that referenced
this pull request
Jun 14, 2018
We've been setting this value to 500ms in the default low-level REST client configuration, misunderstanding the effect that it would have. This proved very problematic, as it ends up causing `TimeoutException` returned from the leased pool in some cases even for successful requests. Closes #24069
idlira
added a commit
to scireum/sirius-db
that referenced
this pull request
Jul 8, 2024
Should be fixed in elastic/elasticsearch#30384 Fixes: OX-11111
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.
We've been setting this value to 500ms in the default low-level REST
client configuration, misunderstanding the effect that it would have.
This proved very problematic, as it ends up causing
TimeoutExceptionreturned from the leased pool in some cases even for successful requests.
Closes #24069