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

server: can't hook initialization steps past cluster settings propagation #17032

Closed
knz opened this issue Jul 15, 2017 · 3 comments
Closed

server: can't hook initialization steps past cluster settings propagation #17032

knz opened this issue Jul 15, 2017 · 3 comments
Labels
A-kv-server Relating to the KV-level RPC server A-server-architecture Relates to the internal APIs and src org for server code C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-server-and-security DB Server & Security

Comments

@knz
Copy link
Contributor

knz commented Jul 15, 2017

This is stalling progress on e.g. #17020: I can't find a place in the code to put an initialization steps that runs 1) after the cluster settings are loaded / propagated via gossip but 2) before the server is ready for client connections.

There should be a hook in the code, clearly documented as such, where one can add initialization code.

@dt suggests:

I'd probably go for the nonblocking write to a chan struct{} in settingsworker.go, the add a read (maybe w/timeout) to the startup path in server (maybe wrapped in a helper). could also go with a waitgroup if you made sure to only call Done the first time, or a cond var? not a perf-sensitive code path, so I would probably take simple over fast.

Unfortunately I was not myself able to translate this into code.

@knz knz added the C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) label Jul 15, 2017
@knz knz assigned dt Jul 15, 2017
@bdarnell
Copy link
Contributor

Potentially related to reworking of the server startup flow in #16371

@tbg tbg added this to the Later milestone Jul 22, 2018
@tbg tbg added the A-kv-server Relating to the KV-level RPC server label Aug 21, 2018
@petermattis petermattis removed this from the Later milestone Oct 5, 2018
@dt dt removed their assignment Oct 17, 2018
@github-actions
Copy link

github-actions bot commented Jun 9, 2021

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

@jlinder jlinder added the T-server-and-security DB Server & Security label Jun 16, 2021
@knz knz added the A-server-architecture Relates to the internal APIs and src org for server code label Jul 29, 2021
@ajwerner
Copy link
Contributor

I believe this is stale. We load settings before accepting client connections now IIUC. At least in secondary tenants this definitely happens and I'm working on unifying the behavior in #69269. I don't think this issue is providing anybody with value so I'm going to close it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-kv-server Relating to the KV-level RPC server A-server-architecture Relates to the internal APIs and src org for server code C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-server-and-security DB Server & Security
Projects
None yet
Development

No branches or pull requests

7 participants