Skip to content

Commit 49b02e1

Browse files
committed
Modernize opensearchtransport with Go 1.23 idioms
Use slices.Backward for reverse iteration in pool dead-list removal. Replace manual wg.Add/go/defer patterns with sync.WaitGroup.Go in tests. Signed-off-by: Sean Chittenden <sean.chittenden@crowdstrike.com>
1 parent ca24043 commit 49b02e1

6 files changed

Lines changed: 49 additions & 31 deletions

File tree

opensearchapi/api_scroll_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
package opensearchapi_test
1010

1111
import (
12+
"context"
1213
"strings"
1314
"testing"
1415
"time"
@@ -29,7 +30,7 @@ func TestScrollClient(t *testing.T) {
2930

3031
testIndex := testutil.MustUniqueString(t, "test-scroll")
3132
t.Cleanup(func() {
32-
client.Indices.Delete(t.Context(), opensearchapi.IndicesDeleteReq{Indices: []string{testIndex}})
33+
client.Indices.Delete(context.Background(), opensearchapi.IndicesDeleteReq{Indices: []string{testIndex}})
3334
})
3435

3536
_, err = client.Document.Create(

opensearchapi/api_tasks_test.go

Lines changed: 34 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"strconv"
1515
"strings"
1616
"testing"
17+
"time"
1718

1819
"github.com/stretchr/testify/assert"
1920
"github.com/stretchr/testify/require"
@@ -28,6 +29,9 @@ func TestTasksClient(t *testing.T) {
2829
t.Parallel()
2930
client, err := testutil.NewClient(t)
3031
require.NoError(t, err)
32+
// .tasks index mapping lacked cancellation_time_millis and resource_stats fields;
33+
// cancelled tasks could never persist completion (opensearch-project/OpenSearch#16201).
34+
testutil.SkipIfVersion(t, client, "<", "2.18", "TestTasksClient")
3135
failingClient, err := osapitest.CreateFailingClient(t)
3236
require.NoError(t, err)
3337

@@ -120,16 +124,6 @@ func TestTasksClient(t *testing.T) {
120124

121125
// Create unique indices for this test
122126
destIndex := testutil.MustUniqueString(t, "test-tasks-dest")
123-
testIndices := []string{sourceIndex, destIndex}
124-
t.Cleanup(func() {
125-
client.Indices.Delete(
126-
context.Background(),
127-
opensearchapi.IndicesDeleteReq{
128-
Indices: testIndices,
129-
Params: opensearchapi.IndicesDeleteParams{IgnoreUnavailable: opensearchapi.ToPointer(true)},
130-
},
131-
)
132-
})
133127

134128
// Create destination index
135129
client.Indices.Create(
@@ -157,6 +151,21 @@ func TestTasksClient(t *testing.T) {
157151
require.NotEmpty(t, resp)
158152
taskID := resp.Task
159153

154+
t.Cleanup(func() {
155+
client.Tasks.Cancel(context.Background(), opensearchapi.TasksCancelReq{TaskID: taskID})
156+
require.Eventually(t, func() bool {
157+
resp, err := client.Tasks.Get(context.Background(), opensearchapi.TasksGetReq{TaskID: taskID})
158+
return err == nil && resp.Completed
159+
}, 30*time.Second, 100*time.Millisecond, "reindex task did not complete after cancel")
160+
client.Indices.Delete(
161+
context.Background(),
162+
opensearchapi.IndicesDeleteReq{
163+
Indices: []string{destIndex},
164+
Params: opensearchapi.IndicesDeleteParams{IgnoreUnavailable: opensearchapi.ToPointer(true)},
165+
},
166+
)
167+
})
168+
160169
testCases := []tasksTests{
161170
{
162171
Name: "with request",
@@ -204,16 +213,6 @@ func TestTasksClient(t *testing.T) {
204213

205214
// Create unique indices for this test
206215
destIndex := testutil.MustUniqueString(t, "test-tasks-dest")
207-
testIndices := []string{sourceIndex, destIndex}
208-
t.Cleanup(func() {
209-
client.Indices.Delete(
210-
context.Background(),
211-
opensearchapi.IndicesDeleteReq{
212-
Indices: testIndices,
213-
Params: opensearchapi.IndicesDeleteParams{IgnoreUnavailable: opensearchapi.ToPointer(true)},
214-
},
215-
)
216-
})
217216

218217
// Create destination index
219218
client.Indices.Create(
@@ -241,6 +240,21 @@ func TestTasksClient(t *testing.T) {
241240
require.NotEmpty(t, resp)
242241
taskID := resp.Task
243242

243+
t.Cleanup(func() {
244+
client.Tasks.Cancel(context.Background(), opensearchapi.TasksCancelReq{TaskID: taskID})
245+
require.Eventually(t, func() bool {
246+
resp, err := client.Tasks.Get(context.Background(), opensearchapi.TasksGetReq{TaskID: taskID})
247+
return err == nil && resp.Completed
248+
}, 30*time.Second, 100*time.Millisecond, "reindex task did not complete after cancel")
249+
client.Indices.Delete(
250+
context.Background(),
251+
opensearchapi.IndicesDeleteReq{
252+
Indices: []string{destIndex},
253+
Params: opensearchapi.IndicesDeleteParams{IgnoreUnavailable: opensearchapi.ToPointer(true)},
254+
},
255+
)
256+
})
257+
244258
testCases := []tasksTests{
245259
{
246260
Name: "with request",

opensearchtransport/connection_warmup_internal_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -695,9 +695,7 @@ func TestWarmupConcurrentSkip(t *testing.T) {
695695

696696
const goroutines = 16
697697
for range goroutines {
698-
wg.Add(1)
699-
go func() {
700-
defer wg.Done()
698+
wg.Go(func() {
701699
for {
702700
switch conn.tryWarmupSkip() {
703701
case warmupSkipped:
@@ -708,7 +706,7 @@ func TestWarmupConcurrentSkip(t *testing.T) {
708706
return
709707
}
710708
}
711-
}()
709+
})
712710
}
713711

714712
wg.Wait()

opensearchtransport/opensearchtransport.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1611,11 +1611,17 @@ func (c *Client) signRequest(req *http.Request) error {
16111611
}
16121612

16131613
func (c *Client) setReqUserAgent(req *http.Request) {
1614+
if req.Header == nil {
1615+
req.Header = make(http.Header, 1)
1616+
}
16141617
req.Header.Set("User-Agent", c.userAgent)
16151618
}
16161619

16171620
func (c *Client) setReqGlobalHeader(req *http.Request) {
16181621
if len(c.header) > 0 {
1622+
if req.Header == nil {
1623+
req.Header = make(http.Header, len(c.header))
1624+
}
16191625
for k, v := range c.header {
16201626
if req.Header.Get(k) != k {
16211627
for _, vv := range v {

opensearchtransport/pool_multi_server.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"context"
1111
"math/rand/v2"
1212
"net/url"
13+
"slices"
1314
"sort"
1415
"sync"
1516
"sync/atomic"
@@ -595,8 +596,8 @@ func (cp *multiServerPool) removeFromReadyWithLock(c *Connection) {
595596
func (cp *multiServerPool) removeFromDeadWithLock(c *Connection) {
596597
idx := -1
597598
// Search backward -- recently appended items are at the tail
598-
for i := len(cp.mu.dead) - 1; i >= 0; i-- {
599-
if cp.mu.dead[i] == c {
599+
for i, v := range slices.Backward(cp.mu.dead) {
600+
if v == c {
600601
idx = i
601602
break
602603
}

opensearchtransport/selector_round_robin_internal_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,7 @@ func TestRoundRobinSelectorConcurrency(t *testing.T) {
123123

124124
// Start multiple goroutines selecting connections
125125
for range numGoroutines {
126-
wg.Add(1)
127-
go func() {
128-
defer wg.Done()
126+
wg.Go(func() {
129127
for range selectionsPerGoroutine {
130128
selected, err := selector.Select(connections)
131129
if err != nil {
@@ -134,7 +132,7 @@ func TestRoundRobinSelectorConcurrency(t *testing.T) {
134132
}
135133
results <- selected
136134
}
137-
}()
135+
})
138136
}
139137

140138
wg.Wait()

0 commit comments

Comments
 (0)