habsi/elasticsearch-js
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Drop default 30-second timeout (#2573)

Browse Source
This commit is contained in:
Josh Mock
2025-01-30 10:37:53 -06:00
committed by GitHub
parent 6dbf91a9c3
commit 0ad42ff1a2
10 changed files with 48 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
docs/basic-config.asciidoc
View File

@ -13,7 +13,6 @@ const client = new Client({
cloud: { id: '<cloud-id>' },
auth: { apiKey: 'base64EncodedKey' },
maxRetries: 5,
requestTimeout: 60000,
sniffOnStart: true
})
----
@ -82,7 +81,7 @@ _Default:_ `3`
|`requestTimeout`
|`number` - Max request timeout in milliseconds for each request. +
_Default:_ `30000`
_Default:_ No value
|`pingTimeout`
|`number` - Max ping request timeout in milliseconds for each request. +

5
docs/changelog.asciidoc
View File

@ -12,6 +12,11 @@
In 8.0, the top-level `body` parameter that was available on all API functions <<remove-body-key,was deprecated>>. In 9.0 this property is completely removed.
[discrete]
===== Remove the default 30-second timeout on all requests sent to Elasticsearch
Setting HTTP timeouts on Elasticsearch requests goes against Elastic's recommendations. See <<timeout-best-practices>> for more information.
[discrete]
=== 8.17.0

23
docs/child.asciidoc
View File

@ -1,22 +1,22 @@
[[child]]
=== Creating a child client
There are some use cases where you may need multiple instances of the client.
You can easily do that by calling `new Client()` as many times as you need, but
you will lose all the benefits of using one single client, such as the long
living connections and the connection pool handling. To avoid this problem, the
client offers a `child` API, which returns a new client instance that shares the
There are some use cases where you may need multiple instances of the client.
You can easily do that by calling `new Client()` as many times as you need, but
you will lose all the benefits of using one single client, such as the long
living connections and the connection pool handling. To avoid this problem, the
client offers a `child` API, which returns a new client instance that shares the
connection pool with the parent client.
NOTE: The event emitter is shared between the parent and the child(ren). If you
extend the parent client, the child client will have the same extensions, while
NOTE: The event emitter is shared between the parent and the child(ren). If you
extend the parent client, the child client will have the same extensions, while
if the child client adds an extension, the parent client will not be extended.
You can pass to the `child` every client option you would pass to a normal
client, but the connection pool specific options (`ssl`, `agent`, `pingTimeout`,
You can pass to the `child` every client option you would pass to a normal
client, but the connection pool specific options (`ssl`, `agent`, `pingTimeout`,
`Connection`, and `resurrectStrategy`).
CAUTION: If you call `close` in any of the parent/child clients, every client
CAUTION: If you call `close` in any of the parent/child clients, every client
will be closed.
[source,js]
@ -28,9 +28,8 @@ const client = new Client({
})
const child = client.child({
headers: { 'x-foo': 'bar' },
requestTimeout: 1000
})
client.info().then(console.log, console.log)
child.info().then(console.log, console.log)
----
----

4
docs/connecting.asciidoc
View File

@ -414,8 +414,8 @@ The supported request specific options are:
_Default:_ `null`
|`requestTimeout`
|`number | string` - Max request timeout for the request in milliseconds, it overrides the client default. +
_Default:_ `30000`
|`number | string | null` - Max request timeout for the request in milliseconds. This overrides the client default, which is to not time out at all. See https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-network.html#_http_client_configuration[Elasticsearch best practices for HTML clients] for more info. +
_Default:_ No timeout
|`retryOnTimeout`
|`boolean` - Retry requests that have timed out.

8
docs/timeout-best-practices.asciidoc
View File

@ -1,10 +1,8 @@
[[timeout-best-practices]]
=== Timeout best practices
This client is configured by default to operate like many HTTP client libraries do, by using a relatively short (30 second) timeout on all requests sent to {es}, raising a `TimeoutError` when that time period has elapsed without receiving a response. However, {es} will always eventually respond to any request, even if it takes several minutes. The {ref}/modules-network.html#_http_client_configuration[official {es} recommendation] is to disable response timeouts entirely by default.
Starting in 9.0.0, this client is configured to not time out any HTTP request by default. {es} will always eventually respond to any request, even if it takes several minutes. Reissuing a request that it has not responded to yet can cause performance side effects. See the {ref}/modules-network.html#_http_client_configuration[official {es} recommendations for HTTP clients] for more information.
Since changing this default would be a breaking change, we won't do that until the next major release. In the meantime, here is our recommendation for properly configuring your client:
Prior to 9.0, this client was configured by default to operate like many HTTP client libraries do, by using a relatively short (30 second) timeout on all requests sent to {es}, raising a `TimeoutError` when that time period elapsed without receiving a response.
* Ensure keep-alive is enabled; this is the default, so no settings need to be changed, unless you have set `agent` to `false` or provided an alternate `agent` that disables keep-alive
* If using the default `UndiciConnection`, disable request timeouts by setting `timeout` to `0`
* If using the legacy `HttpConnection`, set `timeout` to a very large number (e.g. `86400000`, or one day)
If your circumstances require you to set timeouts on Elasticsearch requests, setting the `requestTimeout` value to a millisecond value will cause this client to operate as it did prior to 9.0.

4
package.json
View File

@ -57,7 +57,7 @@
},
"devDependencies": {
"@elastic/request-converter": "8.17.0",
"@sinonjs/fake-timers": "github:sinonjs/fake-timers#48f089f",
"@sinonjs/fake-timers": "14.0.0",
"@types/debug": "4.1.12",
"@types/ms": "0.7.34",
"@types/node": "22.10.7",
@ -89,7 +89,7 @@
"zx": "7.2.3"
},
"dependencies": {
"@elastic/transport": "^8.9.1",
"@elastic/transport": "9.0.0-alpha.1",
"apache-arrow": "^18.0.0",
"tslib": "^2.4.0"
},

6
src/client.ts
View File

@ -108,14 +108,15 @@ export interface ClientOptions {
* @defaultValue 3 */
maxRetries?: number
/** @property requestTimeout Max request timeout in milliseconds for each request
* @defaultValue 30000 */
* @defaultValue No timeout
* @remarks Read [the Elasticsearch docs](https://www.elastic.co/guide/en/elasticsearch/reference/current/modules-network.html#_http_client_configuration) about HTTP client configuration for details. */
requestTimeout?: number
/** @property pingTimeout Max number of milliseconds a `ClusterConnectionPool` will wait when pinging nodes before marking them dead
* @defaultValue 3000 */
pingTimeout?: number
/** @property sniffInterval Perform a sniff operation every `n` milliseconds
* @remarks Sniffing might not be the best solution for you. Read https://www.elastic.co/blog/elasticsearch-sniffing-best-practices-what-when-why-how to learn more.
* @defaultValue */
* @defaultValue false */
sniffInterval?: number | boolean
/** @property sniffOnStart Perform a sniff once the client is started
* @remarks Sniffing might not be the best solution for you. Read https://www.elastic.co/blog/elasticsearch-sniffing-best-practices-what-when-why-how to learn more.
@ -244,7 +245,6 @@ export default class Client extends API {
Serializer,
ConnectionPool: (opts.cloud != null) ? CloudConnectionPool : WeightedConnectionPool,
maxRetries: 3,
requestTimeout: 30000,
pingTimeout: 3000,
sniffInterval: false,
sniffOnStart: false,

28
test/unit/client.test.ts
View File

@ -17,9 +17,10 @@
* under the License.
*/
import * as http from 'http'
import * as http from 'node:http'
import { URL } from 'node:url'
import { setTimeout } from 'node:timers/promises'
import { test } from 'tap'
import { URL } from 'url'
import FakeTimers from '@sinonjs/fake-timers'
import { buildServer, connection } from '../utils'
import { Client, errors } from '../..'
@ -451,30 +452,37 @@ test('Ensure new client instance stores requestTimeout for each connection', t =
t.end()
})
test('Ensure new client does not time out at default (30s) when client sets requestTimeout', async t => {
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'] })
test('No request timeout is set by default', t => {
const client = new Client({
node: { url: new URL('http://localhost:9200') },
})
t.equal(client.connectionPool.connections[0].timeout, null)
t.end()
})
test('Ensure new client does not time out if requestTimeout is not set', async t => {
const clock = FakeTimers.install({ toFake: ['setTimeout'] })
t.teardown(() => clock.uninstall())
function handler (_req: http.IncomingMessage, res: http.ServerResponse) {
setTimeout(() => {
t.pass('timeout ended')
setTimeout(1000 * 60 * 60).then(() => {
t.ok('timeout ended')
res.setHeader('content-type', 'application/json')
res.end(JSON.stringify({ success: true }))
}, 31000) // default is 30000
clock.runToLast()
})
clock.tick(1000 * 60 * 60)
}
const [{ port }, server] = await buildServer(handler)
const client = new Client({
node: `http://localhost:${port}`,
requestTimeout: 60000
})
try {
await client.transport.request({ method: 'GET', path: '/' })
} catch (error) {
t.fail('timeout error hit')
t.fail('Error should not be thrown', error)
} finally {
server.stop()
t.end()

4
test/unit/helpers/bulk.test.ts
View File

@ -1611,7 +1611,7 @@ test('errors', t => {
test('Flush interval', t => {
t.test('Slow producer', async t => {
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'] })
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'], shouldClearNativeTimers: true })
t.teardown(() => clock.uninstall())
let count = 0
@ -1663,7 +1663,7 @@ test('Flush interval', t => {
})
t.test('Abort operation', async t => {
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'] })
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'], shouldClearNativeTimers: true })
t.teardown(() => clock.uninstall())
let count = 0

2
test/unit/helpers/msearch.test.ts
View File

@ -583,7 +583,7 @@ test('Multiple searches (concurrency = 1)', t => {
test('Flush interval', t => {
t.plan(2)
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'] })
const clock = FakeTimers.install({ toFake: ['setTimeout', 'clearTimeout'], shouldClearNativeTimers: true })
t.teardown(() => clock.uninstall())
const MockConnection = connection.buildMockConnection({