kadm: do not reuse ApiVersions in many concurrent requests

twmb · twmb · commit 1955938745ea · 2023-10-21T12:29:43.000-06:00
The client calls SetVersion internally per request, so doing this
concurrently leads to races.
diff --git a/pkg/kadm/acls.go b/pkg/kadm/acls.go
@@ -924,7 +924,7 @@ func (cl *Client) DescribeACLs(ctx context.Context, b *ACLBuilder) (DescribeACLs
 	)
 	defer cancel()
 	for i := range descs {
-		req := descs[i]
+		req := descs[i] // each req is unique per loop, we are not reusing req, this is safe
 		myIdx := i
 		wg.Add(1)
 		go func() {
diff --git a/pkg/kadm/misc.go b/pkg/kadm/misc.go
@@ -244,10 +244,6 @@ func (cl *Client) ApiVersions(ctx context.Context) (BrokersApiVersions, error) {
 		return nil, err
 	}
 
-	req := kmsg.NewPtrApiVersionsRequest()
-	req.ClientSoftwareName = "kadm"
-	req.ClientSoftwareVersion = softwareVersion()
-
 	var mu sync.Mutex
 	var wg sync.WaitGroup
 	vs := make(BrokersApiVersions, len(m.Brokers))
@@ -256,6 +252,9 @@ func (cl *Client) ApiVersions(ctx context.Context) (BrokersApiVersions, error) {
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
+			req := kmsg.NewPtrApiVersionsRequest()
+			req.ClientSoftwareName = "kadm"
+			req.ClientSoftwareVersion = softwareVersion()
 			v := BrokerApiVersions{NodeID: n, keyVersions: make(map[int16]minmax)}
 			v.raw, v.Err = req.RequestWith(ctx, cl.cl.Broker(int(n)))
 

Original file line number	Diff line number	Diff line change
`@@ -924,7 +924,7 @@ func (cl Client) DescribeACLs(ctx context.Context, b ACLBuilder) (DescribeACLs`
`924`	`924`	`)`
`925`	`925`	`defer cancel()`
`926`	`926`	`for i := range descs {`
`927`		`- req := descs[i]`
	`927`	`+ req := descs[i] // each req is unique per loop, we are not reusing req, this is safe`
`928`	`928`	`myIdx := i`
`929`	`929`	`wg.Add(1)`
`930`	`930`	`go func() {`