Skip to content

Commit 20192a7

Browse files
committed
docs(buffering): rewrite guide for Do[T] vs Stream
Rewrite guides/response_buffering.md around the two entry points: opensearch.Do[T] (typed, buffered, default; SDK owns the body) versus opensearchtransport.Client.Stream (raw, unbuffered, caller owns the body). Document the reason there is intentionally no typed streaming helper, and update the proxy example to use client.Stream(req). CHANGELOG: replace the DisableResponseBuffering "Added" entry with the Stream/Client.Stream "Added" entry; add a Deprecated entry for Perform on both opensearchtransport.Client and opensearch.Client (removal in v5); drop the stale DisableResponseBuffering reference from the opensearch-project#859 Fixed entry. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 1d645c2 commit 20192a7

2 files changed

Lines changed: 67 additions & 32 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1818
- Add `OPENSEARCH_GO_ROUTER` environment variable to enable the DefaultRouter without code changes; set to `true` to opt in (off by default in v4, on by default in v5, removed in v6) ([#815](https://github.com/opensearch-project/opensearch-go/pull/815))
1919
- Add client-side metrics guide covering Metrics API, ConnectionMetric, PolicySnapshot, and RouterSnapshot ([#812](https://github.com/opensearch-project/opensearch-go/pull/812))
2020
- Add `InsecureSkipVerify` config option to disable TLS certificate verification without constructing a custom `http.Transport`, preserving `DefaultTransport` connection pooling, HTTP/2, and timeout defaults ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
21-
- Add `DisableResponseBuffering` config option to skip eager `io.ReadAll` buffering of response bodies in `Perform()`, reducing per-request allocations and TTFB for proxy and streaming use cases ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
21+
- Add `(*opensearchtransport.Client).Stream(*http.Request) (*http.Response, error)` and a `(*opensearch.Client).Stream` passthrough for raw byte forwarding (proxy and streaming use cases). Stream returns the unbuffered response body from `RoundTrip`; the caller owns reading and closing `res.Body`. Pairs with `opensearch.Do[T]` for typed, decoded results (the SDK owns the body). Stream is exposed only on the concrete `*Client` in v4; v5 will add it to `opensearchtransport.Interface` and remove the deprecated `Perform` ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
2222
- Add per-attempt `RequestTimeout` to bound individual HTTP round-trips, preventing indefinite hangs on stalled connections ([#786](https://github.com/opensearch-project/opensearch-go/issues/786))
2323
- Add `opensearchutil/shardhash` package with exported `Hash` and `ForRouting` functions for computing OpenSearch shard routing
2424
- Enhanced cluster readiness checking for improved test reliability: `testutil.NewClient()` now includes readiness validation (health + cluster state + nodes info)
@@ -201,6 +201,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
201201

202202
### Deprecated
203203

204+
- Mark `opensearchtransport.Client.Perform` and the `opensearch.Client.Perform` passthrough as deprecated; both remain fully functional in v4 (still buffering the response body via `io.ReadAll` + `NopCloser`) and will be removed in v5. New code should call `opensearch.Do[T]` for typed, decoded results or `opensearchtransport.Client.Stream` / `opensearch.Client.Stream` for raw byte forwarding.
204205
- Mark `Client.Do()` with a `Deprecated` doc annotation in favor of `opensearch.Do[T]()` for compile-time pointer safety; `Client.Do()` remains fully functional and will not be removed, but `staticcheck` SA1019 will nudge cross-package callers toward the safer generic alternative
205206
- Mark `opensearch.ToPointer` and `opensearchapi.ToPointer` as deprecated; they remain fully functional but will be removed in v5. Once the module's go directive moves to 1.26, callers can drop the helper entirely in favor of native `new(value)` literal syntax (e.g. `new(false)`)
206207

@@ -212,7 +213,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
212213
- Fix `opensearchtransport.Client.setReqGlobalHeader` comparing the per-request header value against the global header name, so a request-level header never suppressed the matching global default and both were sent ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
213214
- Fix gzip buffer-pool nil poisoning on compress error: `gzipCompressor.compress` returned `(nil, err)` while the caller's deferred `collectBuffer` still ran, putting a typed-nil `*bytes.Buffer` into the `sync.Pool` that panics on the next `Get().Reset()` ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
214215
- Fix `opensearchtransport.Client.Perform` silently dropping `io.ReadAll` errors during response buffering via `:=` shadowing; the read error now propagates wrapped in the new `opensearchtransport.ErrResponseBodyRead` sentinel, and `opensearch.Client.Do` classifies the `(resp != nil, err != nil)` case via `errors.Is` so only genuine body-read failures are labeled `ErrReadBody` (an unrelated transport error returned alongside a response, such as context cancellation during retry backoff, is no longer misreported as a read failure) ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
215-
- Fix error-response body not being closed in `opensearch.ParseError` and the v5preview `do()` no-decode path; under `DisableResponseBuffering` this lets `http.Transport` reuse the connection on error responses (in the default buffered mode `Perform` already drains and closes the body, so the connection is already reusable). The v5preview no-decode path now reads the body to EOF and re-wraps it in a `NopCloser` rather than discarding it, keeping the returned `resp.Body` readable and consistent with the `ParseError` path ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
216+
- Fix error-response body not being closed in `opensearch.ParseError` and the v5preview `do()` no-decode path. The v5preview no-decode path now reads the body to EOF and re-wraps it in a `NopCloser` rather than discarding it, keeping the returned `resp.Body` readable and consistent with the `ParseError` path ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
216217
- Fix response-body lifecycle on the raw `RoundTrip` paths that lack `Perform`'s buffering safety net, where closing a partially-read body defeated HTTP keep-alive: the stats poller (`cluster_health.go`) and discovery's `/_cat/shards`, `/_cluster/state/metadata`, and `/_nodes` paths now drain to EOF before close via a shared `drainAndClose` helper (covering both non-200 returns and `json.Decode` success paths that stop before EOF). `opensearch.Response.String()` is now non-consuming -- it restores `Body` with an in-memory reader after rendering, so logging a response no longer empties a body other code expects to read. The AWS v1 and v2 signers now close the request body on the read-error path in `hexEncodedSha256OfRequest` ([#859](https://github.com/opensearch-project/opensearch-go/pull/859))
217218
- Add typed response-format defaults for `v5preview/opensearchapi/` cat, list, ppl, and sql operations: when the caller leaves `Format` unset, the SDK now emits the value the typed Resp struct expects (`json` for cat/list/explain, `jdbc` for ppl/sql query) instead of letting the server fall back to a default the JSON decoder cannot handle.
218219
- Replace `WaitForAllNodesReady` inline `require.Eventually` loop with a layered readiness FSM (`internal/test/readiness`) that observes per-node progression through `LayerTCP -> LayerHTTP -> LayerClusterJoin -> LayerStatsReady`, records transitions including regressions, and emits a structured per-node diagnostic with the full last cat-nodes response on timeout. Per-layer budgets are tuned for CI pessimism (cold JVM startup is the long pole); total budget for `TargetClusterReady` is 6.5 minutes. ([#650](https://github.com/opensearch-project/opensearch-go/issues/650))

guides/response_buffering.md

Lines changed: 64 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,66 @@
1-
# Response Body Buffering
1+
# Response Body Lifecycle: `Do[T]` vs `Stream`
22

3-
By default, the OpenSearch Go client reads and buffers the entire HTTP response body in `Perform()` before returning it to the caller. This guarantees that the underlying TCP connection is drained and returned to the connection pool, even if the caller does not fully read the body.
3+
The OpenSearch Go client exposes two entry points for issuing requests, each with a different response-body ownership contract. Pick the one that matches your use case; do not mix them.
44

5-
For most use cases this is the right behavior. However, for **proxy** and **streaming** workloads where the caller forwards large responses incrementally, buffering adds memory pressure and increases time-to-first-byte (TTFB) because the entire body must be read before the caller sees any bytes.
5+
| Entry point | Body ownership | Buffering | Use when |
6+
| ----------------------------------- | -------------- | -------------------- | ------------------------------------------------------------------------ |
7+
| `opensearch.Do[T]` | SDK | Buffered (in memory) | You want a typed, decoded Go value (CRUD, search, cluster ops). Default. |
8+
| `opensearchtransport.Client.Stream` | Caller | Unbuffered (raw) | You want to forward or relay raw bytes downstream (proxy, streaming). |
69

7-
## Disabling Response Buffering
10+
There is intentionally no typed streaming helper. "Stream and decode into `T`" is a contradiction: if you want `T`, use `Do[T]`; if you want bytes, use `Stream`.
811

9-
Set `DisableResponseBuffering: true` to skip the buffering step. Perform() returns the raw `http.Response.Body` from the underlying `RoundTrip`, and the **caller is responsible for fully reading and closing it**.
12+
## `Do[T]`: typed, buffered, default
1013

11-
```go
12-
client, err := opensearch.NewClient(opensearch.Config{
13-
Addresses: []string{"https://localhost:9200"},
14-
Username: "admin",
15-
Password: "myStrongPassword123!",
14+
`opensearch.Do[T]` (and the per-API `do(...)` helpers in `opensearchapi`, `v5preview/opensearchapi`, and `plugins/*`) call into `opensearchtransport.Client.Perform`, which:
15+
16+
1. Reads the entire response body into memory.
17+
2. Closes the underlying body.
18+
3. Replaces `Response.Body` with an `io.NopCloser` over a `bytes.Reader`.
19+
20+
This guarantees the underlying TCP connection is drained and returned to the connection pool even if the caller never reads the body, and lets the SDK decode the buffered bytes into a Go value.
1621

17-
// Skip response body buffering — the caller will stream the body.
18-
DisableResponseBuffering: true,
22+
```go
23+
client, err := opensearchapi.NewClient(opensearchapi.Config{
24+
Client: opensearch.Config{
25+
Addresses: []string{"https://localhost:9200"},
26+
Username: "admin",
27+
Password: "myStrongPassword123!",
28+
},
1929
})
30+
if err != nil {
31+
return err
32+
}
33+
34+
resp, err := client.Cluster.Health(ctx, &opensearchapi.ClusterHealthReq{})
35+
if err != nil {
36+
return err
37+
}
38+
fmt.Println(resp.Status)
2039
```
2140

22-
### Proxy example
41+
## `Stream`: raw, unbuffered, caller owns the body
42+
43+
`opensearchtransport.Client.Stream` returns the raw `*http.Response` from the underlying `http.RoundTripper`. The SDK does not read or close `res.Body`; the caller does. Stream still performs routing, retries, signing, header injection, request-body compression, and the seed URL fallback identically to `Perform`.
44+
45+
`opensearch.Client` exposes a `Stream` passthrough so callers do not need to type-assert `c.Transport`:
46+
47+
```go
48+
res, err := client.Stream(req)
49+
if err != nil {
50+
return err
51+
}
52+
defer res.Body.Close()
53+
// io.Copy / decode incrementally / forward bytes downstream...
54+
```
55+
56+
If the underlying transport is a custom implementation that does not satisfy the `opensearch.Streamer` interface, `client.Stream` returns `opensearch.ErrTransportMissingMethodStream`.
57+
58+
### Proxy and streaming example
2359

2460
A reverse proxy can use `io.Copy` (or `io.CopyBuffer` with a pooled buffer) to stream responses to downstream clients with minimal memory overhead:
2561

2662
```go
27-
res, err := client.Perform(req)
63+
res, err := client.Stream(req)
2864
if err != nil {
2965
http.Error(w, err.Error(), http.StatusBadGateway)
3066
return
@@ -37,32 +73,30 @@ for k, v := range res.Header {
3773
}
3874
w.WriteHeader(res.StatusCode)
3975

40-
// Stream the body bytes flow to the client as they arrive from OpenSearch.
76+
// Stream the body: bytes flow to the client as they arrive from OpenSearch.
4177
if _, err := io.Copy(w, res.Body); err != nil {
4278
log.Printf("stream copy error: %v", err)
4379
}
4480
```
4581

46-
## Connection Reuse
82+
### Connection reuse with `Stream`
4783

48-
The buffering exists to ensure HTTP/1.1 connections are properly drained and returned to the pool. When buffering is disabled:
84+
Because `Stream` does not buffer, the caller is responsible for the body lifecycle:
4985

50-
- **HTTP/2**: Streams are multiplexed on a single connection, so draining is not required for connection reuse. This is the recommended protocol when disabling buffering.
51-
- **HTTP/1.1**: The caller **must** fully read the response body before the connection can be reused. If the caller abandons a partially-read body, the connection will be closed rather than returned to the pool.
86+
- **HTTP/2**: streams are multiplexed on a single connection, so draining is not required for connection reuse.
87+
- **HTTP/1.1**: the caller MUST fully read the response body before the connection can be reused. If the caller abandons a partially-read body, the connection will be closed rather than returned to the pool.
5288

5389
In both cases, always call `res.Body.Close()` when done.
5490

55-
## When to Use
91+
## When to use which
5692

57-
| Scenario | Recommendation |
58-
| -------------------------------------------------- | --------------------------------- |
59-
| Standard API calls (CRUD, search, cluster ops) | Leave buffering enabled (default) |
60-
| Reverse proxy forwarding large responses | Disable buffering |
61-
| Streaming bulk responses to clients | Disable buffering |
62-
| Scroll/PIT with large result sets piped downstream | Disable buffering |
93+
| Scenario | Recommendation |
94+
| -------------------------------------------------- | -------------- |
95+
| Standard API calls (CRUD, search, cluster ops) | `Do[T]` |
96+
| Reverse proxy forwarding large responses | `Stream` |
97+
| Streaming bulk responses to clients | `Stream` |
98+
| Scroll/PIT with large result sets piped downstream | `Stream` |
6399

64-
## Configuration Reference
100+
## Deprecation note
65101

66-
| Field | Type | Default | Location |
67-
| -------------------------- | ------ | ------- | ------------------------------------------------- |
68-
| `DisableResponseBuffering` | `bool` | `false` | `opensearch.Config`, `opensearchtransport.Config` |
102+
`opensearchtransport.Client.Perform` and `opensearch.Client.Perform` are marked deprecated in v4 and will be removed in v5. They remain fully functional for v4 callers; the buffered-response contract is unchanged. New code should call `Do[T]` for typed results or `Stream` for raw byte forwarding.

0 commit comments

Comments
 (0)