You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
docs(changelog): correct the site counts and the bulk indexer exclusion
The counts predated the bulk indexer fixups: 153 sites in
opensearchtransport and 39 elsewhere. The exclusion also described the
wrong case. A client the caller supplies is never owned by the indexer,
so the surviving unclosed site is the one that forces implicitClient to
true and asserts the indexer closed it. Note that the poller guard
carries !integration for the same reason the allocation assertions do.
Signed-off-by: Ryan Yuan <ryan.yuan@crowdstrike.com>
Copy file name to clipboardExpand all lines: CHANGELOG.md
+1-1Lines changed: 1 addition & 1 deletion
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -255,7 +255,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
255
255
256
256
### Fixed
257
257
258
-
- Fix tests, benchmarks, and examples that construct a transport or client and never release it, each leaking two ticker goroutines that outlive the test. `opensearchtransport.New` always starts the node-stats poller and the cluster-health refresh loop: `healthCheckRate` is derived from the server core count and is never zero, and `NodeStatsInterval: 0` means auto-derive rather than disabled. `Close` is the only thing that stops either one, so a leaked poller keeps ticking for the remaining life of the test binary, where it perturbs process-wide measurements. Every site now registers a release -- `t.Cleanup` in tests, `b.Cleanup` in benchmarks, `defer` in examples -- covering 151 sites in `opensearchtransport` and 26 more across `opensearch_integration_test.go`, `opensearch_benchmark_test.go`, `opensearch_example_test.go`, `opensearchapi`, `opensearchutil`, `osprom`, and `osotel`. Three kinds of site are deliberately left alone: the process-wide shared client from `opensearchapi/testutil.NewClient`, which the package owns rather than the caller; a `New` whose construction is expected to fail and returns nothing to close; and a bulk indexer that created its own client, since `BulkIndexer.Close` already releases it. The two zero-allocation assertions (`TestClassify_ZeroAlloc` and `TestNewRequestEventZeroAlloc`) move into `//go:build !integration` files: `testing.AllocsPerRun` is a process-wide allocation differential and is only sound in a binary where nothing else allocates concurrently, and sharing a binary with the live-cluster tests is what flaked `TestClassify_ZeroAlloc` in CI. New `TestCloseReapsBackgroundPollers` reads the goroutine dump to assert both pollers start with `New` and are gone after `Close`, so the leak cannot return silently
258
+
- Fix tests, benchmarks, and examples that construct a transport or client and never release it, each leaking two ticker goroutines that outlive the test. `opensearchtransport.New` always starts the node-stats poller and the cluster-health refresh loop: `healthCheckRate` is derived from the server core count and is never zero, and `NodeStatsInterval: 0` means auto-derive rather than disabled. `Close` is the only thing that stops either one, so a leaked poller keeps ticking for the remaining life of the test binary, where it perturbs process-wide measurements. Every site now registers a release -- `t.Cleanup` in tests, `b.Cleanup` in benchmarks, `defer` in examples -- covering 153 sites in `opensearchtransport` and 39 more across `opensearch_integration_test.go`, `opensearch_benchmark_test.go`, `opensearch_example_test.go`, `opensearchapi`, `opensearchutil`, `osprom`, and `osotel`. Three kinds of site are deliberately left alone: the process-wide shared client from `opensearchapi/testutil.NewClient`, which the package owns rather than the caller; a `New` whose construction is expected to fail and returns nothing to close; and one bulk indexer case that forces `implicitClient` to true so the indexer owns the client and `BulkIndexer.Close` releases it. A client the caller supplies is not owned, as `TestBulkIndexerOwnClientFlag` asserts, so the cases that hand the indexer a client close it themselves. The two zero-allocation assertions (`TestClassify_ZeroAlloc` and `TestNewRequestEventZeroAlloc`) move into `//go:build !integration` files: `testing.AllocsPerRun` is a process-wide allocation differential and is only sound in a binary where nothing else allocates concurrently, and sharing a binary with the live-cluster tests is what flaked `TestClassify_ZeroAlloc` in CI. New `TestCloseReapsBackgroundPollers` reads the goroutine dump to assert both pollers start with `New` and are gone after `Close`, so the leak cannot return silently; it carries the same `!integration` constraint, because the dump only settles in a binary where no other live transport is polling
259
259
- Fix `cmd/osgen` dropping every version annotation the spec writes beside a `$ref`. kin-openapi splits a `$ref`'s siblings across two places: standard fields such as `description` are overlaid onto the resolved schema, but `x-*` keys stay on the reference and never reach the resolved schema's extensions. The generator read only the latter, so 141 annotations were lost -- 135 `x-version-added`, 5 `x-version-removed`, and 1 `x-version-deprecated`. The visible half was missing documentation: `SearchResp.PhaseTook` carries `x-version-added: '2.12'` and emitted no availability note, and no generated file mentioned that version at all. The other half is a correctness problem, since the same values feed the version filter, so those fields were tested against an empty version and could not be excluded by `-min-version` or `-max-version`. A sibling annotation now wins over one on the referenced schema, because it describes the property carrying it rather than the shared type it points at: two properties may reference one schema and have been added in different versions. Regenerating adds 138 availability notes and changes no field
260
260
- Fix collapsed types keeping their mangled generic-instantiation name instead of the readable alias the spec provides for them. When an `allOf` adds nothing to its base the two describe one Go type and the base's name was kept, so `AsAdjacencyMatrix()` returned `CommonAggregationsMultiBucketAggregateBaseAdjacencyMatrixBucket` even though the spec supplies `AdjacencyMatrixAggregate` as a bare `allOf: [$ref]` alias precisely to name that instantiation. A post-walk pass now renames the collapsed type to its alias and rewrites every reference, including types keyed beneath it (a nested `buckets` union is registered as `<parentKey>.buckets`, so it inherited the old prefix). The rename must run after the walk rather than during it: type references are plain Go type strings, and the spec chains these collapses (`RangeAggregate` -> `RangeAggregateBase` -> `MultiBucketAggregateBaseRangeBucket`), so a mid-walk rename leaves siblings that already resolved pointing at a name that no longer exists. Two guards keep it safe: a target several aliases share keeps its own name, since no one alias is the better choice (eight schemas from `AvgAggregate` to `WeightedAvgAggregate` collapse onto `SingleMetricAggregateBase`), and a target the spec references more heavily than its alias also keeps its name, so `SearchResult` is not retired in favor of `SearchResponse`. Restores `CommonAggregationsAdjacencyMatrixAggregate`, `CommonAggregationsDateHistogramAggregate`, `CommonAggregationsGeoHashGridAggregate` and their siblings, and drops type names over 60 characters from 74 to 16 -- the remainder being genuinely descriptive nested paths rather than erasure artifacts
261
261
- Fix `cmd/osgen` deciding union branch reachability in the wrong pipeline phase, and stop emitting wrapper structs for schemas that merely rename another. Branch deduplication ran during the Parse phase, dropping any branch whose Go type duplicated an earlier one. Whether a duplicate is dead depends on the union's decode state, which is not assigned until the IR phase: a wire-decoded union walks its branches and stops at the first that decodes, so a same-type duplicate is unreachable, but a caller-keyed lazy union retains only raw bytes and lets the caller name the branch, so every `As<Branch>()` accessor is reachable even when several decode one Go type. Deduplication moves to `dropUnreachableBranches`, which runs once every union has reached its terminal state and skips the lazy ones. `SearchResultAggregationsValue` gains back the accessors the Parse-phase drop had been silently deleting (55 -> 62), including `AsSum`, `AsMin`, `AsMax`, `AsValueCount`, `AsWeightedAvg`, `AsSimpleValue`, and `AsMedianAbsoluteDeviation` alongside `AsAvg`. With reachability now judged correctly, `collapsesToBase` also accepts a bare `allOf: [$ref]` -- the spec's way of giving a generic instantiation a friendly name -- which removes 30 further wrapper structs whose only content was the embedded base (66 such wrappers at the start of this line of work, 2 remain). Breaking: the removed wrappers are no longer distinct types, so `CommonAggregationsAvgAggregate` and its siblings are now `CommonAggregationsSingleMetricAggregateBase`, and `New...FromAvg` and friends take that type; accessor and constructor names are unchanged
0 commit comments