cli,rpc: don't check the active cluster version in the CLI#75237
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Jan 21, 2022
Merged
cli,rpc: don't check the active cluster version in the CLI#75237craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
Member
|
This change is |
andreimatei
reviewed
Jan 20, 2022
Contributor
andreimatei
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andreimatei and @knz)
pkg/rpc/context.go, line 925 at r1 (raw file):
if rpcCtx.ClientOnly { // client-only RPC contexts don't have a node ID to report nor a // cluster version to check against the remote node.
nit: the version checking is not particularly tied to the "remote node". I'd scratch that part.
5bf9c0b to
f3dc87d
Compare
knz
commented
Jan 20, 2022
Contributor
Author
knz
left a comment
There was a problem hiding this comment.
TFYR!
bors r=andreimatei
Reviewable status:
pkg/rpc/context.go, line 925 at r1 (raw file):
Previously, andreimatei (Andrei Matei) wrote…
nit: the version checking is not particularly tied to the "remote node". I'd scratch that part.
Thanks, removed.
Contributor
Author
|
bors r- |
Contributor
|
Canceled. |
Contributor
Author
|
let's wait for CI to pass before merging this |
This commit removes a code path that would tickle an assertion failure if we were to later fix the context propagation in the RPC heartbeat method (see PR cockroachdb#71243): there's no "active cluster version" in the CLI and so we can't compare it in a client interceptor. Release note: None
f3dc87d to
ad6289a
Compare
Contributor
Author
|
bors r=andreimatei |
Contributor
|
Build succeeded: |
craig bot
pushed a commit
that referenced
this pull request
Jan 21, 2022
71243: rpc: add logging tags for rpc connect events r=andreimatei a=knz Epic: CRDB-11517 First commit from #75237. This ensures that the log messages sent to the DEV channel relating to RPC low-level activity (e.g. conn failures, heartbeat etc) include the remote node ID and address and connection class. This also connects the heartbeat task to the tracing infra properly. Example before: ``` [n6] 87 dialing n4: 127.0.0.1:41706 (default) ``` After: ``` [n6,rnode=4,raddr=127.0.0.1:41706,class=default] 87 dialing ``` 74333: kvserver/loqrecovery: write replica recovery events to rangelog r=tbg,erikgrinaker a=aliher1911 Previously a fact of loss of quorum replica recovery was only written as a structured log entry. This information is local to the node and does not survive if node is decommissioned. It would be beneficial to preserve this information longer. Range log while being limited to 30 days still provide a good reference data for investigations if recovery wasn't performed too long ago. This patch adds entries to rangelog for every updated range first time node that holds survivor replica is started after recovery. Release note: None Addresses #73679 Co-authored-by: Raphael 'kena' Poss <knz@thaumogen.net> Co-authored-by: Oleg Afanasyev <oleg@cockroachlabs.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit removes a code path that would tickle an assertion failure
if we were to later fix the context propagation in the RPC heartbeat
method (see PR #71243): there's no "active cluster version" in the CLI
and so we can't compare it in a client interceptor.
Release note: None