Skip to content

Commit f4893ee

Browse files
fix: fixes a data race I added changing this to a map (#16814)
1 parent f55d0c2 commit f4893ee

File tree

1 file changed

+21
-13
lines changed

1 file changed

+21
-13
lines changed

‎pkg/limits/frontend/ring.go

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,24 +48,24 @@ func (g *RingStreamUsageGatherer) GetStreamUsage(ctx context.Context, r GetStrea
4848

4949
// TODO(grobinson): Need to rename this to something more accurate.
5050
func (g *RingStreamUsageGatherer) forAllBackends(ctx context.Context, r GetStreamUsageRequest) ([]GetStreamUsageResponse, error) {
51-
replicaSet, err := g.ring.GetAllHealthy(LimitsRead)
51+
rs, err := g.ring.GetAllHealthy(LimitsRead)
5252
if err != nil {
5353
return nil, err
5454
}
55-
return g.forGivenReplicaSet(ctx, replicaSet, r)
55+
return g.forGivenReplicaSet(ctx, rs, r)
5656
}
5757

58-
func (g *RingStreamUsageGatherer) forGivenReplicaSet(ctx context.Context, replicaSet ring.ReplicationSet, r GetStreamUsageRequest) ([]GetStreamUsageResponse, error) {
59-
partitions, err := g.getConsumedPartitions(ctx, replicaSet)
58+
func (g *RingStreamUsageGatherer) forGivenReplicaSet(ctx context.Context, rs ring.ReplicationSet, r GetStreamUsageRequest) ([]GetStreamUsageResponse, error) {
59+
partitions, err := g.getConsumedPartitions(ctx, rs)
6060
if err != nil {
6161
return nil, err
6262
}
6363

6464
errg, ctx := errgroup.WithContext(ctx)
65-
responses := make([]GetStreamUsageResponse, len(replicaSet.Instances))
65+
responses := make([]GetStreamUsageResponse, len(rs.Instances))
6666

6767
// TODO: We shouldn't query all instances since we know which instance holds which stream.
68-
for i, instance := range replicaSet.Instances {
68+
for i, instance := range rs.Instances {
6969
errg.Go(func() error {
7070
client, err := g.pool.GetClientFor(instance.Addr)
7171
if err != nil {
@@ -93,11 +93,17 @@ func (g *RingStreamUsageGatherer) forGivenReplicaSet(ctx context.Context, replic
9393
return responses, nil
9494
}
9595

96-
func (g *RingStreamUsageGatherer) getConsumedPartitions(ctx context.Context, replicaSet ring.ReplicationSet) (map[string][]int32, error) {
96+
type getAssignedPartitionsResponse struct {
97+
Addr string
98+
Response *logproto.GetAssignedPartitionsResponse
99+
}
100+
101+
func (g *RingStreamUsageGatherer) getConsumedPartitions(ctx context.Context, rs ring.ReplicationSet) (map[string][]int32, error) {
97102
errg, ctx := errgroup.WithContext(ctx)
98-
responses := make(map[string]*logproto.GetAssignedPartitionsResponse)
103+
responses := make([]getAssignedPartitionsResponse, len(rs.Instances))
104+
99105
// Get the partitions assigned to each instance.
100-
for _, instance := range replicaSet.Instances {
106+
for i, instance := range rs.Instances {
101107
errg.Go(func() error {
102108
client, err := g.pool.GetClientFor(instance.Addr)
103109
if err != nil {
@@ -107,7 +113,9 @@ func (g *RingStreamUsageGatherer) getConsumedPartitions(ctx context.Context, rep
107113
if err != nil {
108114
return err
109115
}
110-
responses[instance.Addr] = resp
116+
// No need for a mutex here as responses is a "Structured variable"
117+
// as described in https://go.dev/ref/spec#Variables.
118+
responses[i] = getAssignedPartitionsResponse{Addr: instance.Addr, Response: resp}
111119
return nil
112120
})
113121
}
@@ -122,11 +130,11 @@ func (g *RingStreamUsageGatherer) getConsumedPartitions(ctx context.Context, rep
122130
// with the latest timestamp.
123131
highestTimestamp := make(map[int32]int64)
124132
assigned := make(map[int32]string)
125-
for addr, resp := range responses {
126-
for partition, assignedAt := range resp.AssignedPartitions {
133+
for _, resp := range responses {
134+
for partition, assignedAt := range resp.Response.AssignedPartitions {
127135
if t := highestTimestamp[partition]; t < assignedAt {
128136
highestTimestamp[partition] = assignedAt
129-
assigned[partition] = addr
137+
assigned[partition] = resp.Addr
130138
}
131139
}
132140
}

0 commit comments

Comments
 (0)