physicalplan: debugging for segfault in fakeSpanResolverIterator.Seek #100204
Conversation
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @cucaroach)
There was a problem hiding this comment.
Reviewable status:
pkg/sql/physicalplan/fake_span_resolver.go line 171 at r1 (raw file):
fit.ranges = make([]fakeRange, len(splits)-1) for i := range fit.ranges { nodes := fit.fsr.nodes
minor nit: maybe there should be a comment here so that a reasonable person doesn't clean this up in the near future.
There was a problem hiding this comment.
Reviewable status:
pkg/sql/physicalplan/fake_span_resolver.go line 172 at r1 (raw file):
for i := range fit.ranges { nodes := fit.fsr.nodes n := len(nodes)
It looks weird (to me) that n and nodes aren't hoisted outside the loop.
Break up a line that is segfaulting into several lines, so that we can tell which part is to blame if it happens again. Informs: cockroachdb#100051 Informs: cockroachdb#100108 Epic: None Release note: None
There was a problem hiding this comment.
TFTRs!
bors r=rharding6373,cucaroach
Reviewable status:
pkg/sql/physicalplan/fake_span_resolver.go line 171 at r1 (raw file):
Previously, rharding6373 (Rachael Harding) wrote…
minor nit: maybe there should be a comment here so that a reasonable person doesn't clean this up in the near future.
Done.
pkg/sql/physicalplan/fake_span_resolver.go line 172 at r1 (raw file):
Previously, cucaroach (Tommy Reilly) wrote…
It looks weird (to me) that n and nodes aren't hoisted outside the loop.
Done.
|
This PR was included in a batch that was canceled, it will be automatically retried |
|
Build failed (retrying...): |
|
Build succeeded: |
|
might be worth backporting? in case this happens to 23.1 deployments |
blathers backport 23.1 |
Break up a line that is segfaulting into several lines, so that we can tell which part is to blame if it happens again.
Informs: #100051
Informs: #100108
Epic: None
Release note: None