span: Re-initialize iterator when forwarding #115509
Conversation
|
This change is |
miretskiy
force-pushed
the
spanfrontier
branch
from
a27240b to
8c5e86d
Compare
Use concurrent frontier in TestRangeFeedMetricsManagement. Fixes cockroachdb#115407 Release note: None
miretskiy
force-pushed
the
spanfrontier
branch
from
8c5e86d to
bce833d
Compare
pkg/util/span/llrb_frontier.go
Outdated
| if err := collectOverlaps(span, startAt, f, &sg); err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err := collectOverlaps(span, startAt, f, &sg); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Why do we do this twice? If required, it could definitely use a comment.
If unintentional, perhaps collectOverlaps could construct the SpanGroup and return it rather than taking it as an out param?
pkg/util/span/frontier.go
Outdated
| // collectOverlaps collects all spans overlapping s, and including s, into span | ||
| // group. Any overlapping spans with timestamp less than startAt are forwarded. | ||
| func collectOverlaps( |
There was a problem hiding this comment.
I wonder if we could give this a name that makes it more clear it will mutate the given frontier.
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @stevendanna)
pkg/util/span/frontier.go line 822 at r2 (raw file):
Previously, stevendanna (Steven Danna) wrote…
I wonder if we could give this a name that makes it more clear it will mutate the given frontier.
Got better name; and simplified this function and the call sites.
This function, now called spanDifference simply computes a set difference
between the span and the frontier, and returns that diff.
pkg/util/span/llrb_frontier.go line 135 at r2 (raw file):
Previously, stevendanna (Steven Danna) wrote…
Why do we do this twice? If required, it could definitely use a comment.
If unintentional, perhaps
collectOverlapscould construct the SpanGroup and return it rather than taking it as an out param?
Total copy pasta.
Re-initialize iterator when forwarding span frontier timestamp. The underlying btree may be mutated (by merge operation) invalidating previously constructed iterator. Fixes cockroachdb#115411 Release notes: None
miretskiy
force-pushed
the
spanfrontier
branch
from
bce833d to
e571747
Compare
|
bors r+ |
|
Build succeeded: |
Re-initialize iterator when forwarding span
frontier timestamp. The underlying btree may be
mutated (by merge operation) invalidating previously
constructed iterator.
Btree implementation is also hardened against mis-use
when mutating span frontier while iterating.
Fixes #115411
Fixes #115528
Fixes #115512
Fixes #115490
Fixes #115488
Fixes #115487
Fixes #115483
Release notes: None