Skip to content

Commit ca3b71a

Browse files
ryanyuansean-
andauthored
feat(opensearch)!: add Client.Close() and cache implicit default clients (#893)
Constructing a v4+ client spawns background goroutines (node discovery, health/stats pollers, DNS refresh) plus a connection pool that are released only when the transport is closed. Callers that build-and-discard clients -- most acutely NewBulkIndexer building a default client per loop -- leaked them for the process lifetime, with no way to tear one down without type-asserting the transport. Add explicit teardown and share implicitly-created default clients: - Close() on opensearch.Client and opensearchapi.Client. Cancels the background goroutines and closes idle connections via an io.Closer check, so a custom Interface without Close is a safe no-op. Safe on a zero value and idempotent. - A process-wide, refcounted, idle-TTL cache (internal/ttlcache) for implicitly-constructed default clients -- NewDefaultClient on both packages and the client NewBulkIndexer builds when none is supplied. Identical default configs resolve to one shared transport (and its goroutines and pool) instead of one set per construction, keyed by a hash of the config. User-built NewClient clients never enter the cache; un-hashable configs bypass it and build fresh. A cache hit mints a thin per-holder Client whose Close decrements the shared refcount; the eviction worker closes the transport once no holder remains and it goes idle. ttlcache is transport-agnostic: it caches over a caller-supplied constructor and liveness probe, with a CAS-claim protocol arbitrating the lock-free hit path against the eviction sweep. - NewBulkIndexer closes the client it implicitly creates when the indexer is closed, on every exit path including a cancelled context. Its periodic flusher is stopped via context cancellation so Close never deadlocks. Tune the idle eviction window with OPENSEARCH_GO_DEFAULT_CLIENT_TTL: a time.ParseDuration string ("16m") or a bare number of seconds ("30", "1.5"); default 16m (the 15m AWS Lambda max plus a 1m buffer), 0 never evicts, a negative value disables caching. BREAKING: two NewDefaultClient calls with identical config now share one transport, so their Metrics() counters are aggregated across all holders rather than per-client. Callers that built multiple default clients to read separate metrics should switch to NewClient (never cached) or disable caching via the env var. See UPGRADING_V5.md. Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com> Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com> Co-authored-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 60dd3ef commit ca3b71a

18 files changed

Lines changed: 1741 additions & 108 deletions

CHANGELOG.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
66

77
### Added
88

9+
- Add `Close()` to `opensearch.Client` and `opensearchapi.Client` for explicit teardown of background goroutines (node discovery, health/stats pollers, DNS refresh) and idle connections, without type-asserting the transport. Cache implicitly-constructed default clients (`opensearch.NewDefaultClient`, `opensearchapi.NewDefaultClient`, and the client `opensearchutil.NewBulkIndexer` builds when none is supplied) in a process-wide, refcounted, idle-TTL cache keyed by config hash, so identical default clients share one transport instead of leaking one set of goroutines and its connection pool per construction. User-built `opensearch.NewClient`/`opensearchapi.NewClient` clients never enter the cache. `opensearchutil.NewBulkIndexer` now closes the client it implicitly creates when the indexer is closed. Tune the idle eviction window with `OPENSEARCH_GO_DEFAULT_CLIENT_TTL` (default `16m`; `0` = never evict; a negative value disables caching so every call builds a fresh client) ([#893](https://github.com/opensearch-project/opensearch-go/issues/893))
910
- Add client-side DNS caching, enabled by default on the built-in transport. Resolved addresses are cached and re-resolved on an interval (default 60s, mirroring the TTL AWS publishes for managed OpenSearch Service endpoints). When the resolver becomes briefly unreachable, the last-known-good address continues to be served until the resolver recovers, so transient resolver outages (e.g. a node-local DNS blip producing `dial tcp: lookup ...: i/o timeout`) no longer fail requests for already-resolved hosts. Tune or disable via the `DNSCacheRefresh`, `DNSDialTimeout`, `DNSKeepAlive`, and `DNSTimeout` fields on `opensearch.Config` (or `OPENSEARCH_GO_DNS_CACHE_REFRESH`, `OPENSEARCH_GO_DNS_DIAL_TIMEOUT`, `OPENSEARCH_GO_DNS_KEEP_ALIVE`, `OPENSEARCH_GO_DNS_TIMEOUT`); each follows the 0 = default, <0 = disable, >0 = explicit convention. Caching is installed only when no custom `Transport` is supplied; a caller-provided `Transport` is never modified. A host that resolves to multiple addresses races up to three of them concurrently (random start offset per connection) and takes the first to connect, spreading load and tolerating a dead address. Refresh re-resolves cached hosts sequentially, so `DNSTimeout` (default 10s) bounds each lookup to keep one hung resolution from stalling a refresh tick. The refresh goroutine is bound to the client's root context, so it is reclaimed both when `Close` is called and when `New` returns an error after the context is created. Because Go's resolver does not expose record TTLs, the refresh interval is a re-resolution cadence, not a per-record TTL. Exposes `DNSLookups`, `DNSCacheMisses`, and `DNSLookupErrors` counters via `Transport.Metrics()`
1011
- `cmd/osgen`: guard `json.RawMessage` in generated request/response types behind a checked-in allowlist (`cmd/osgen/rawmessage_allowlist.txt`). Because a `json.RawMessage` is the symptom of a type the generator could not resolve, a generator bug can silently widen the raw-JSON surface of the public API; generation now fails (non-zero exit) when any `json.RawMessage` use is not listed, including nested forms such as `[]json.RawMessage`, `map[string]json.RawMessage`, and `[][]json.RawMessage` (the leaf is detected at any wrapper depth). Entries are keyed `GoTypeName/jsonFieldName` (whole-response raw bodies use `<Prefix>Resp/-`, and map/array responses whose element type is unresolved use `<Prefix>Resp/[entries]` and `<Prefix>Resp/[records]`). Add `-update-raw-message-allowlist` to regenerate the allowlist from current output (sorted and grouped for minimal diffs), and `-allow-unlisted-raw-message` to downgrade the check to a warning ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
1112
- `cmd/osgen`: emit int-backed (const `iota`) enum types for string fields carrying an `x-enum-name` marker alongside an `enum:` constraint. Each enum generates a named int type with a zero-value `<Name>Unknown` sentinel, name<->value lookup maps, `String()`, `MarshalJSON`, and a closed-set `UnmarshalJSON` that rejects unknown wire values via a typed `*Unknown<Name>Error` (recoverable through `errors.As`). The marker is shared, so a single enum type is registered once and reused across every referencing field; a marker reused with a conflicting value set fails generation rather than silently merging. Applied to the security `status` field, which becomes a typed `RestStatus` enum ([#890](https://github.com/opensearch-project/opensearch-go/pull/890))
1213
- Add `OPENSEARCH_GO_POLICY_DUMP` environment variable: when set with `OPENSEARCH_GO_DEBUG=true`, dumps the router's policy tree (the dot-delimited node paths that `OPENSEARCH_GO_POLICY_*` matchers target, each labeled with its pool or role) to the debug logger at client initialization. The dump walks the structural tree so router wrappers that share an inner policy instance are each rendered in full. ([#883](https://github.com/opensearch-project/opensearch-go/issues/883))
1314
- Add a `build-samples` Makefile target and a CI job that compiles and vets every `_samples/*.go` program, so example breakage is caught (the `_samples` directory is excluded from `go build ./...` because Go ignores `_`-prefixed paths)
1415
- Group document operations under a `client.Doc` sub-client and point-in-time operations under `client.PIT` (`Create`/`Delete`/`GetAll`/`DeleteAll`); `client.Document` and `client.PointInTime` remain as field aliases. The indices sub-client's canonical field is `client.Index`, with `client.Indices` and `client.Indexes` as aliases. `cmd/osgen` gains `--emit-v4-compat` (default true) to emit backward-compatibility forwarders so top-level `client.Bulk`/`MGet`/`Update`, `client.Document.Source`, and `client.PointInTime.Get` keep working (`client.Index` is not forwarded -- it is the indices sub-client field; use `client.Doc.Index`), and `--emit-v4-deprecation` (default false) to mark those forwarders deprecated
1516
- Add `cmd/osgen` code generator for typed path builders and API consumer files from the OpenAPI spec
16-
- `opensearchapi`: `NewClient` and `NewDefaultClient` inject `opensearchtransport.NewDefaultRouter` when `config.Client.Router` is nil, opting every client into intelligent request routing by default. The `OPENSEARCH_GO_ROUTER` env var controls the behavior: `=false`/`=0` suppresses both Router injection and auto-discovery; unset or any other value injects the Router and enables on-start discovery. ([#816](https://github.com/opensearch-project/opensearch-go/issues/816))
17-
- Add `envvars.Falsy(name)` helper that distinguishes "explicitly opted out" from "unset" (Truthy collapses both into false). Used by the router injection rule.
17+
- The built-in transport builds `opensearchtransport.NewDefaultRouter` when `Config.Router` is nil, and `opensearch.NewClient` enables on-start discovery under the same condition, so a client routes across discovered nodes by default. `OPENSEARCH_GO_ROUTER` controls it: `=false`/`=0` suppresses both the default router and on-start discovery; unset or any other value builds the router and enables discovery. ([#816](https://github.com/opensearch-project/opensearch-go/issues/816))
18+
- Add `envvars.Falsy(name)`, which tells "explicitly opted out" apart from "unset" (Truthy treats both as false). The default-router rule in the transport and `opensearch.NewClient` use it.
1819
- Add the code-generated `opensearchapi/` package: API surface produced by `cmd/osgen` from the OpenAPI spec. Fully typed Req/Resp/Params structs, sub-clients matching OpenSearch namespaces (`client.Cat`, `client.Cluster`, `client.Indices`, etc.), and a `plugins/` subtree for ML/k-NN/security/ISM/etc. Replaces the hand-written v4 package (previewed in the v4 line at `v5preview/opensearchapi/`); see `opensearchapi/README.md` for usage and `UPGRADING.md` for migration guidance ([#650](https://github.com/opensearch-project/opensearch-go/issues/650))
1920
- Add `primary_terms_map` and `split_shards_metadata` fields to ClusterState index metadata for OpenSearch >=3.6.0 compatibility
2021
- Add address resolver handler to rewrite discovered node addresses before they enter the connection pool ([#822](https://github.com/opensearch-project/opensearch-go/pull/822))
@@ -168,6 +169,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
168169

169170
- **BREAKING**: Per-request transport metrics (`requests`, `failures`, responses-by-status) are now always collected via lock-free atomics, independent of `EnableMetrics`. `EnableMetrics` now gates only the detailed-metrics snapshot (per-connection, per-policy, and router state returned by `Metrics()`). The responses-by-status counter moved from a mutex-guarded map to a lock-free atomic array. `Metrics()` no longer returns an error when metrics are disabled -- it always returns the per-request counters (callers that branched on `if err != nil` for the disabled case should drop that check). See [`UPGRADING_V5.md`](UPGRADING_V5.md#metrics-error-on-disabled-removed) for migration. ([#891](https://github.com/opensearch-project/opensearch-go/issues/891))
170171
- Make the detailed-metrics snapshot path lock-free at call time. The per-connection `deadSince`/`overloadedAt` timestamps moved from `Connection.mu`-guarded `time.Time` fields to lock-free atomic Unix-nanosecond values, so `Metrics()` enumerates connections without taking each connection's mutex. Under concurrent request load this was the dominant lock-contention site (a mutex profile attributed ~3.85% of total contention delay to the snapshot reader taking a write lock merely to read two fields); the conversion drops that to ~0.1%. Writes still occur under `Connection.mu` so the resurrection/standby read-modify-write decisions stay serialized. Benchmarks (`BenchmarkMetrics`, `BenchmarkMetricsParallel`, `BenchmarkMetricsUnderLoad`) confirm the always-on detailed path is acceptable. ([#892](https://github.com/opensearch-project/opensearch-go/issues/892))
172+
- **BREAKING**: Implicitly-created default clients are now cached and shared. Two `opensearch.NewDefaultClient` (or `opensearchapi.NewDefaultClient`) calls with identical config resolve to one shared transport instead of two independent ones, so they share goroutine and connection-pool lifecycle and their `Metrics()` counters are aggregated across all holders rather than per-client. A caller that built multiple default clients to read separate metrics will now see combined counts. To keep independent transports, build with `opensearch.NewClient`/`opensearchapi.NewClient` (never cached) or set `OPENSEARCH_GO_DEFAULT_CLIENT_TTL` to a negative value to disable caching. See [`UPGRADING_V5.md`](UPGRADING_V5.md#default-client-caching) for migration. ([#893](https://github.com/opensearch-project/opensearch-go/issues/893))
171173
- Reorganize the documentation. Split `UPGRADING.md` into a version-history index plus per-major-version guides (`UPGRADING_V5.md` through `UPGRADING_V2.md`) and rename `opensearchapi/MIGRATING.md` to `opensearchapi/UPGRADING_V4_TO_V5.md`. Group the `guides/` and `_samples/` files by subsystem (`transport-`, `indexing-`, `usage-`, `config-`) and add a `guides/README.md` index. Make `guides/usage-error_handling.md` the single source for partial-error handling and `guides/transport-retry_backoff.md` the single source for resurrection-timeout config, replacing the duplicated copies in `opensearchapi/README.md` and `guides/transport-routing.md` with links. Add package documentation (`doc.go`) for `opensearchapi`, `plugins`, `signer`, and `signer/awsv2`.
172174
- Trim the CI compatibility matrix to the currently-patched OpenSearch set (2.19.x and 3.x) per the 12-month support policy; older lines (1.3.x - 2.18.x) are no longer part of the tested matrix and the 4.x client remains their supported path. No client code change ([#856](https://github.com/opensearch-project/opensearch-go/issues/856))
173175
- **BREAKING**: Module path is now `github.com/opensearch-project/opensearch-go/v5`. Update import paths from `/v4` to `/v5`; the in-source `opensearchapi.X` package qualifier is unchanged

UPGRADING_V5.md

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,3 +255,25 @@ cfg := opensearch.Config{
255255
```
256256

257257
The same knobs are available via environment variables (`OPENSEARCH_GO_DNS_CACHE_REFRESH=-1` to disable), and `DNSDialTimeout` / `DNSKeepAlive` tune the underlying dialer. A caller-supplied `Transport` is never modified, so any client that sets its own `Transport` is unaffected and opts out implicitly. Because Go's resolver does not expose record TTLs, the interval is a re-resolution cadence rather than a per-record TTL.
258+
259+
## Default client caching
260+
261+
v5 caches implicitly-created default clients. `opensearch.NewDefaultClient`, `opensearchapi.NewDefaultClient`, and the client `opensearchutil.NewBulkIndexer` builds when none is supplied now resolve identical configs to one shared, refcounted transport keyed by config hash, instead of constructing an independent transport (and its goroutines and connection pool) per call. v4 built a fresh transport every time.
262+
263+
The observable change is `Metrics()`. Because two default clients with identical config share one transport, their per-request counters are aggregated across every holder rather than isolated per client:
264+
265+
```go
266+
a, _ := opensearch.NewDefaultClient()
267+
b, _ := opensearch.NewDefaultClient() // same config -> same shared transport as a
268+
269+
// a and b now report the combined request/failure counts for both, not each client's own.
270+
```
271+
272+
If you built multiple default clients specifically to read separate metrics, switch to `opensearch.NewClient`/`opensearchapi.NewClient`, which are never cached and always get their own transport:
273+
274+
```go
275+
a, _ := opensearch.NewClient(opensearch.Config{}) // independent transport, isolated metrics
276+
b, _ := opensearch.NewClient(opensearch.Config{}) // independent transport, isolated metrics
277+
```
278+
279+
To turn caching off process-wide, set `OPENSEARCH_GO_DEFAULT_CLIENT_TTL` to a negative value (e.g. `-1` or `-1s`) so every call builds a fresh client. The variable otherwise tunes the idle eviction window and accepts either a `time.ParseDuration` string (`16m`) or a bare number of seconds (`30`, `1.5`); default `16m`, `0` never evicts. Call `Close()` on a default client when done so its shared transport can be reclaimed once no holder remains and it goes idle.

guides/config-envvars.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Every runtime variable, its default, and a one-line summary. Use this as the tab
3131
| [`OPENSEARCH_GO_STANDBY_ROTATION_INTERVAL`](#connection-pool-tuning) | `0` (use discovery interval) | Standby rotation interval |
3232
| [`OPENSEARCH_GO_STANDBY_ROTATION_COUNT`](#connection-pool-tuning) | `1` | Standby rotations per cycle |
3333
| [`OPENSEARCH_GO_STANDBY_PROMOTION_CHECKS`](#connection-pool-tuning) | `3` | Health checks before promotion |
34+
| [`OPENSEARCH_GO_DEFAULT_CLIENT_TTL`](#default-client-cache) | `16m` | Default-client cache idle eviction |
3435
| [`OPENSEARCH_GO_DEBUG`](#debug-and-diagnostics) | `false` | Debug logging |
3536
| [`OPENSEARCH_GO_ERROR_MASK`](#error-masking) | report all (v5+) | Partial-failure category mask |
3637
| [`OPENSEARCH_GO_POLICY_*`](#policy-overrides) | all enabled | Per-policy disable (10 variables) |
@@ -92,6 +93,12 @@ Build, test, and code-generation variables (not read by the client at runtime) a
9293
| `OPENSEARCH_GO_STANDBY_ROTATION_COUNT` | Integer | `1` | Standby connections rotated per cycle. | [Routing: Connection Pool Lifecycle](transport-routing.md#8-connection-pool-lifecycle) |
9394
| `OPENSEARCH_GO_STANDBY_PROMOTION_CHECKS` | Integer | `3` | Consecutive successful health checks required to promote a standby connection to active. | [Routing: Connection Pool Lifecycle](transport-routing.md#8-connection-pool-lifecycle) |
9495

96+
## Default client cache
97+
98+
| Variable | Accepted values | Default | Meaning | See also |
99+
| --- | --- | --- | --- | --- |
100+
| `OPENSEARCH_GO_DEFAULT_CLIENT_TTL` | Duration or seconds | `16m` | Idle eviction window for the process-wide cache of implicitly-created default clients (`opensearch.NewDefaultClient`, `opensearchapi.NewDefaultClient`, and the client `opensearchutil.NewBulkIndexer` builds when none is supplied). Accepts a `time.ParseDuration` string (`16m`) or a bare number of seconds (`30`, `1.5`). Identical default clients share one cached transport, keyed by config hash, until every holder is closed and the entry sits idle for a full window. `0` = never evict (entries live until process exit); a negative value (`-1`, `-1s`) disables caching, so every call builds a fresh client; positive = explicit window. The `16m` default is the 15m AWS Lambda max timeout plus a 1m buffer, so a default client is not evicted mid-invocation across the longest possible Lambda run. Has no `opensearch.Config` equivalent — user-built `opensearch.NewClient`/`opensearchapi.NewClient` clients never enter the cache. | [opensearchapi/README.md Client Creation](../opensearchapi/README.md#client-creation) |
101+
95102
## Debug and diagnostics
96103

97104
| Variable | Accepted values | Default | Meaning | See also |

0 commit comments

Comments
 (0)