Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

kvserver: log receiver snapshot trace on context errors #106742

Merged
merged 1 commit into from
Jul 14, 2023

Conversation

AlexTalks
Copy link
Contributor

@AlexTalks AlexTalks commented Jul 13, 2023

In #106363, the receiver side of snapshots was modified to log the trace of the current context when context errors (such as timeouts or client-side context cancellation) occur. This refactors that change to log these traces whenever there is any context-based errors that occur during receiver-side handling of the MultiRaft/RaftSnapshot streaming RPC. This includes errors while receiving requests or sending responses from the server side of a snapshot - i.e. any time that the traces cannot be collected and returned to the client.

Additionally, testing for these scenarios has been incorporated as well.

Part of: #105820

Release note: None

@AlexTalks AlexTalks requested a review from a team July 13, 2023 02:15
@AlexTalks AlexTalks requested a review from a team as a code owner July 13, 2023 02:15
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@AlexTalks AlexTalks requested a review from knz July 13, 2023 02:16
@AlexTalks
Copy link
Contributor Author

Alternate approach to #106655.

@AlexTalks AlexTalks force-pushed the log_trace_at_rpc branch 3 times, most recently from ac83d72 to ee6e8c1 Compare July 14, 2023 17:47
Copy link
Contributor

@knz knz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, well done!

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @AlexTalks)


pkg/kv/kvserver/store_snapshot.go line 391 at r1 (raw file):

	snapshotCtx := ctx
	ctx, rSp := tracing.EnsureChildSpan(ctx, s.cfg.Tracer(), "receive snapshot data")
	defer rSp.Finish() // Ensure that the tracing span is closed, even if Receive errors

nit: period at end of comment.

In cockroachdb#106363, the receiver side of snapshots was modified to log the trace
of the current context when context errors (such as timeouts or
client-side context cancellation) occur. This refactors that change to
log these traces whenever there is any context-based errors that occur
during receiver-side handling of the MultiRaft/RaftSnapshot streaming RPC.
This includes errors while receiving requests or sending responses from
the server side of a snapshot - i.e. any time that the traces cannot be
collected and returned to the client.

Additionally, testing for these scenarios has been incorporated as well.

Part of: cockroachdb#105820

Release note: None
@AlexTalks
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Jul 14, 2023

Build succeeded:

@craig craig bot merged commit 301fa5c into cockroachdb:master Jul 14, 2023
@AlexTalks AlexTalks deleted the log_trace_at_rpc branch July 18, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants