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

util: setting server.cidr_mapping_url can block startup #130589

Closed
andrewbaptist opened this issue Sep 12, 2024 · 2 comments · Fixed by #130590
Closed

util: setting server.cidr_mapping_url can block startup #130589

andrewbaptist opened this issue Sep 12, 2024 · 2 comments · Fixed by #130590
Labels
branch-release-23.2 Used to mark GA and release blockers, technical advisories, and bugs for 23.2 branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.1.5-rc branch-release-24.2 Used to mark GA and release blockers, technical advisories, and bugs for 24.2 branch-release-24.2.2-rc C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@andrewbaptist
Copy link
Collaborator

andrewbaptist commented Sep 12, 2024

If server.cidr_mapping_url is set, SetOnChange is registered during startup, but the channel is not read from until Start is called. This can result in the startup of a node being blocked.

Jira issue: CRDB-42134

@andrewbaptist andrewbaptist added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Sep 12, 2024
Copy link

blathers-crl bot commented Sep 12, 2024

Hi @andrewbaptist, please add branch-* labels to identify which branch(es) this C-bug affects.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

craig bot pushed a commit that referenced this issue Sep 12, 2024
130459: replica_rac2: remove processorImpl mutex r=sumeerbhola a=pav-kv

This PR removes the `processorImpl` mutex. Almost all `processorImpl` fields are used under `raftMu` (only by methods containing `RaftMuLocked` infix). Exceptions:

- `logTracker`. It has an internal mutex for interaction with log syncs/admissions out of band.
- `leader.rc` has `rcReferenceUpdateMu` for high-contention interaction with out-of-band `AdmitForEval`.
- `leader.pendingAdmittedMu` has a narrow mutex for quick interaction with appends from `RaftTransport`.
- `enabledWhenLeader` is an atomic and doesn't need a mutex.

Related to #129508

130590: util: fix race in cidr startup r=jaylim-crl a=andrewbaptist

Previously if the `server.cidr_mapping_url` was set and a node restarted, there was a race condition where `SetOnChange` for the setting could be called before the `Start` was called. This could result in it blocking while attempting to submit to the channel.

Fixes: #130589

Release note: None

Co-authored-by: Pavel Kalinnikov <[email protected]>
Co-authored-by: Andrew Baptist <[email protected]>
@craig craig bot closed this as completed in d7a70c4 Sep 12, 2024
Copy link

blathers-crl bot commented Sep 12, 2024

Based on the specified backports for linked PR #130590, I applied the following new label(s) to this issue: branch-release-23.2, branch-release-24.1, branch-release-24.1.5-rc, branch-release-24.2, branch-release-24.2.2-rc. Please adjust the labels as needed to match the branches actually affected by this issue, including adding any known older branches.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@blathers-crl blathers-crl bot added branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-23.2 Used to mark GA and release blockers, technical advisories, and bugs for 23.2 branch-release-24.2 Used to mark GA and release blockers, technical advisories, and bugs for 24.2 branch-release-24.1.5-rc branch-release-24.2.2-rc labels Sep 12, 2024
blathers-crl bot pushed a commit that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: #130589

Release note: None
blathers-crl bot pushed a commit that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: #130589

Release note: None
andrewbaptist added a commit to andrewbaptist/cockroach that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: cockroachdb#130589

Release note: None
andrewbaptist added a commit to andrewbaptist/cockroach that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: cockroachdb#130589

Release note: None
andrewbaptist added a commit to andrewbaptist/cockroach that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: cockroachdb#130589

Release note: None
andrewbaptist added a commit to andrewbaptist/cockroach that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: cockroachdb#130589

Release note: None
andrewbaptist added a commit to andrewbaptist/cockroach that referenced this issue Sep 12, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: cockroachdb#130589

Release note: None
andrewbaptist added a commit to andrewbaptist/cockroach that referenced this issue Sep 13, 2024
Previously if the `server.cidr_mapping_url` was set and a node
restarted, there was a race condition where `SetOnChange` for the
setting could be called before the `Start` was called. This could result
in it blocking while attempting to submit to the channel.

Fixes: cockroachdb#130589

Release note: None
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
branch-release-23.2 Used to mark GA and release blockers, technical advisories, and bugs for 23.2 branch-release-24.1 Used to mark GA and release blockers, technical advisories, and bugs for 24.1 branch-release-24.1.5-rc branch-release-24.2 Used to mark GA and release blockers, technical advisories, and bugs for 24.2 branch-release-24.2.2-rc C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant