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: make node startup more principled #30553

Closed
couchand opened this issue Sep 24, 2018 · 2 comments
Closed

server: make node startup more principled #30553

couchand opened this issue Sep 24, 2018 · 2 comments
Labels
A-kv-server Relating to the KV-level RPC server C-investigation Further steps needed to qualify. C-label will change. S-3-productivity Severe issues that impede the productivity of CockroachDB developers.

Comments

@couchand
Copy link
Contributor

Peeled off of discussion on #25771.

Currently pkg/server is a bit messy. In addition to organization questions such as raised in #24989, there are concerns about pkg/server/server particularly. Two 500 line functions (NewServer and server.Start) are together responsible for coordinating the configuration and startup of basically every subsystem of a running node. For the most part they do it by delegating to other packages, but there are still many lines of implementation details in line. In addition, some subsystems rely on shared mutable state, and so uncertainty in initialization can cause hard-to-debug race conditions such as #25771.

Adding new node startup code is fraught: if we don't expect dependencies we'll be lax in where to put it, and even if we are thoughtful about the question it is hard to know where it should go. Ideally we would make these startup dependencies clear.

We should work to simplify the node startup code. Irrelevant details should be encapsulated, exposing a declarative initialization for each subsystem or component, and moving pure networking code into it's own domain. We'd like to make any (known) initialization dependencies explicit in code, but at least document them.

@tschottdorf @BramGruneir @vilterp

@couchand couchand added the A-kv-server Relating to the KV-level RPC server label Sep 24, 2018
@couchand couchand added this to the 2.2 milestone Sep 24, 2018
@couchand couchand added C-investigation Further steps needed to qualify. C-label will change. S-3-productivity Severe issues that impede the productivity of CockroachDB developers. labels Sep 24, 2018
@kenliu kenliu removed this from the 19.1 milestone Jul 18, 2019
@kenliu
Copy link

kenliu commented Jul 18, 2019

this doesn't belong in the Web UI project, reassigning to Core

@irfansharif
Copy link
Contributor

This area has seen a lot of flux lately (most recently #32574), and while we can always continue to improve our organization here, I'm going to close this out.

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 C-investigation Further steps needed to qualify. C-label will change. S-3-productivity Severe issues that impede the productivity of CockroachDB developers.
Projects
None yet
Development

No branches or pull requests

3 participants