Skip to content

Commit fac2160

Browse files
committed
Mark singleServerPool's connection lcActive at construction
singleServerPool's OnSuccess/OnFailure are no-ops, so a connection placed in this pool would otherwise stay at its initial lifecycle (typically lcUnknown / "dead") for the lifetime of the pool. Anything that reads lifecycle state via buildConnectionMetric -- the metrics API, test readiness gates that lens connection health into the readiness FSM, future observers -- would then incorrectly classify the only available connection as not-ready, even after it had served requests successfully. multiServerPool already does the right thing: when it constructs a pool with initial conns it CAS's lcActive on each one (clearing lcUnknown|lcStandby). This change applies the same transition to singleServerPool by routing all four construction sites through a new newSingleServerPool(conn, *metrics) helper that performs the CAS under conn.mu before returning. Behavior is otherwise unchanged. Existing multiServerPool-> singleServerPool demotion paths preserve their connection's existing state (the CAS is a no-op when lcActive is already set). Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent 91dbcd5 commit fac2160

4 files changed

Lines changed: 31 additions & 10 deletions

File tree

opensearchtransport/discovery.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -871,10 +871,7 @@ func (c *Client) createOrUpdateSingleNodePool(readyConnections, deadConnections
871871
metrics = existingSinglePool.metrics
872872
}
873873

874-
return &singleServerPool{
875-
connection: connection,
876-
metrics: metrics,
877-
}
874+
return newSingleServerPool(connection, metrics)
878875
}
879876

880877
// createOrUpdateMultiNodePoolWithLock handles multi-node connection pool creation/updates.

opensearchtransport/opensearchtransport.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -890,7 +890,7 @@ func New(cfg Config) (*Client, error) {
890890
} else {
891891
// Use client-configured timeout settings for the main connection pool
892892
if len(conns) == 1 {
893-
client.mu.connectionPool = &singleServerPool{connection: conns[0]}
893+
client.mu.connectionPool = newSingleServerPool(conns[0], nil)
894894
} else {
895895
poolCtx, poolCancel := context.WithCancel(ctx)
896896
pool := &multiServerPool{
@@ -2385,10 +2385,7 @@ func (c *Client) demoteConnectionPoolWithLock() *singleServerPool {
23852385

23862386
currentPool.mu.RUnlock()
23872387

2388-
return &singleServerPool{
2389-
connection: connection,
2390-
metrics: metrics,
2391-
}
2388+
return newSingleServerPool(connection, metrics)
23922389

23932390
case *singleServerPool:
23942391
// Already a singleServerPool - return unchanged

opensearchtransport/pool_multi_server.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,7 @@ func (cp *multiServerPool) getWarmupParams() (int, int) {
336336
//nolint:unparam // public API; selector kept for backward compatibility
337337
func NewConnectionPool(conns []*Connection, selector Selector) ConnectionPool {
338338
if len(conns) == 1 {
339-
return &singleServerPool{connection: conns[0]}
339+
return newSingleServerPool(conns[0], nil)
340340
}
341341

342342
pool := &multiServerPool{

opensearchtransport/pool_single_server.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,33 @@ type singleServerPool struct {
1616
metrics *metrics
1717
}
1818

19+
// newSingleServerPool constructs a singleServerPool and marks the connection
20+
// as lcActive.
21+
//
22+
// singleServerPool's OnSuccess/OnFailure are no-ops, so a connection placed
23+
// in this pool would otherwise stay at its initial lifecycle (typically
24+
// lcUnknown / "dead") for the lifetime of the pool. Observers that read
25+
// lifecycle state via buildConnectionMetric (test readiness gates, metric
26+
// consumers, etc.) would then incorrectly classify the only available
27+
// connection as not-ready.
28+
//
29+
// The transition mirrors what multiServerPool does for its initial conns:
30+
// set lcActive, clear lcUnknown|lcStandby. Concurrent mutations of those
31+
// position bits are not expected at construction time, but casLifecycle
32+
// handles the race safely if one occurs.
33+
func newSingleServerPool(conn *Connection, m *metrics) *singleServerPool {
34+
if conn != nil {
35+
conn.mu.Lock()
36+
//nolint:errcheck // lock held; only errLifecycleNoop possible
37+
conn.casLifecycle(conn.loadConnState(), 0, lcActive, lcUnknown|lcStandby)
38+
conn.mu.Unlock()
39+
}
40+
return &singleServerPool{
41+
connection: conn,
42+
metrics: m,
43+
}
44+
}
45+
1946
// Compile-time check that singleServerPool implements ConnectionPool.
2047
var _ ConnectionPool = (*singleServerPool)(nil)
2148

0 commit comments

Comments
 (0)