server: use a shared ID container + guarantee log tag#73159
Merged
craig[bot] merged 3 commits intocockroachdb:masterfrom Dec 8, 2021
Merged
server: use a shared ID container + guarantee log tag#73159craig[bot] merged 3 commits intocockroachdb:masterfrom
craig[bot] merged 3 commits intocockroachdb:masterfrom
Conversation
Member
|
This change is |
62a07da to
907857c
Compare
907857c to
df3eed7
Compare
df3eed7 to
42cf0c3
Compare
rimadeodhar
approved these changes
Nov 29, 2021
Collaborator
rimadeodhar
left a comment
There was a problem hiding this comment.
This change 
Reviewed 3 of 8 files at r1, 7 of 7 files at r4, 2 of 2 files at r5, 3 of 3 files at r6, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde)
RaduBerinde
approved these changes
Nov 30, 2021
Member
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/config.go, line 179 at r6 (raw file):
baseCfg := BaseConfig{ Tracer: tr, IDContainer: &c,
[nit] can just do &base.NodeIDContainer{} since we're not using c
42cf0c3 to
d68a26f
Compare
knz
commented
Dec 8, 2021
Contributor
Author
knz
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/server/config.go, line 179 at r6 (raw file):
Previously, RaduBerinde wrote…
[nit] can just do
&base.NodeIDContainer{}since we're not usingc
Done.
d68a26f to
6168592
Compare
This commit ensures that the same data structure can store both SQL instance IDs and Node IDs. The decision is made upon instantiation of the container which of the two types of IDs the container stores. This simplifies the code and paves the road to using a single container in servers for the purpose of identifying the server instance in traces and logs. This also revisits the change in cockroachdb#72607 by switching the tracing/logging prefix `sqli` to `nsql` for SQL instance servers. Release note: None
Release note: None
Prior to this commit, certain components were initialized from the base server config `(BaseConfig).AmbientContext` before the node ID / SQL instance ID container was attached to it via `AddLogTag`. This commit fixes that by instantiating the node ID / SQL instance ID container once in the `BaseConfig`, together with the `AmbientContext`, and binding them together before the config is used as input to initialize other components. Release note: None
6168592 to
17ba7e9
Compare
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
4 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.
All commits but the last from #73158 and prior PRs.
(Reviewers: focus on the last commit)
Uses the infra set up in #73156, as recommended by Radu in this comment.
Informs #58938.
Fixes #72815.
Supersedes #72702.
Prior to this commit, certain components were initialized from the
base server config
(BaseConfig).AmbientContextbefore the node ID /SQL instance ID container was attached to it via
AddLogTag.This commit fixes that by instantiating the node ID / SQL instance ID
container once in the
BaseConfig, together with theAmbientContext, and binding them together before the config is usedas input to initialize other components.