Skip to content

Commit e648f58

Browse files
author
Matt Whelan
committed
spanconfigreporter: fix a bug related to scan restarts
Previously, if more than one span were supplied as an argument, and a scan after the first resulted in a restart, we would discard the results for prior spans. Since we record only negative results, this masked availability issues with those spans. Instead, we build a separate report per span, and merge the results when the scan has completed. This allows for restarts without data loss. Epic: none Release note: None
1 parent d70ca10 commit e648f58

File tree

1 file changed

+17
-9
lines changed

1 file changed

+17
-9
lines changed

pkg/spanconfig/spanconfigreporter/reporter.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -128,13 +128,15 @@ func (r *Reporter) SpanConfigConformance(
128128
return roachpb.SpanConfigConformanceReport{}, err
129129
}
130130

131-
report := roachpb.SpanConfigConformanceReport{}
131+
ret := roachpb.SpanConfigConformanceReport{}
132132
unavailableNodes := make(map[roachpb.NodeID]struct{})
133133

134134
isLiveMap := r.dep.NodeVitalityInterface.ScanNodeVitalityFromCache()
135135
for _, span := range spans {
136+
// Build a separate report per span, so that we can handle resets correctly.
137+
spanReport := roachpb.SpanConfigConformanceReport{}
136138
if err := r.dep.Scan(ctx, int(rangeDescPageSize.Get(&r.settings.SV)),
137-
func() { report = roachpb.SpanConfigConformanceReport{} /* init */ },
139+
func() { spanReport = roachpb.SpanConfigConformanceReport{} /* init */ },
138140
span,
139141
func(descriptors ...roachpb.RangeDescriptor) error {
140142
for _, desc := range descriptors {
@@ -152,21 +154,21 @@ func (r *Reporter) SpanConfigConformance(
152154
return isLive
153155
}, int(conf.GetNumVoters()), int(conf.GetNumNonVoters()))
154156
if !status.Available {
155-
report.Unavailable = append(report.Unavailable,
157+
spanReport.Unavailable = append(spanReport.Unavailable,
156158
roachpb.ConformanceReportedRange{
157159
RangeDescriptor: desc,
158160
Config: conf,
159161
})
160162
}
161163
if status.UnderReplicated || status.UnderReplicatedNonVoters {
162-
report.UnderReplicated = append(report.UnderReplicated,
164+
spanReport.UnderReplicated = append(spanReport.UnderReplicated,
163165
roachpb.ConformanceReportedRange{
164166
RangeDescriptor: desc,
165167
Config: conf,
166168
})
167169
}
168170
if status.OverReplicated || status.OverReplicatedNonVoters {
169-
report.OverReplicated = append(report.OverReplicated,
171+
spanReport.OverReplicated = append(spanReport.OverReplicated,
170172
roachpb.ConformanceReportedRange{
171173
RangeDescriptor: desc,
172174
Config: conf,
@@ -189,7 +191,7 @@ func (r *Reporter) SpanConfigConformance(
189191
c.NumReplicas = conf.NumReplicas
190192
}
191193
if len(overall.SatisfiedBy[i]) < int(c.NumReplicas) {
192-
report.ViolatingConstraints = append(report.ViolatingConstraints,
194+
spanReport.ViolatingConstraints = append(spanReport.ViolatingConstraints,
193195
roachpb.ConformanceReportedRange{
194196
RangeDescriptor: desc,
195197
Config: conf,
@@ -206,7 +208,7 @@ func (r *Reporter) SpanConfigConformance(
206208
c.NumReplicas = conf.GetNumVoters()
207209
}
208210
if len(voters.SatisfiedBy[i]) < int(c.NumReplicas) {
209-
report.ViolatingConstraints = append(report.ViolatingConstraints,
211+
spanReport.ViolatingConstraints = append(spanReport.ViolatingConstraints,
210212
roachpb.ConformanceReportedRange{
211213
RangeDescriptor: desc,
212214
Config: conf,
@@ -219,10 +221,16 @@ func (r *Reporter) SpanConfigConformance(
219221
}); err != nil {
220222
return roachpb.SpanConfigConformanceReport{}, err
221223
}
224+
225+
// Merge the spanReport into the return value.
226+
ret.Unavailable = append(ret.Unavailable, spanReport.Unavailable...)
227+
ret.UnderReplicated = append(ret.UnderReplicated, spanReport.UnderReplicated...)
228+
ret.OverReplicated = append(ret.OverReplicated, spanReport.OverReplicated...)
229+
ret.ViolatingConstraints = append(ret.ViolatingConstraints, spanReport.ViolatingConstraints...)
222230
}
223231

224232
for nid := range unavailableNodes {
225-
report.UnavailableNodeIDs = append(report.UnavailableNodeIDs, int32(nid))
233+
ret.UnavailableNodeIDs = append(ret.UnavailableNodeIDs, int32(nid))
226234
}
227-
return report, nil
235+
return ret, nil
228236
}

0 commit comments

Comments
 (0)