-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Edit httpd balancer member host and port #365
Conversation
worker->s->port, 0, | ||
worker->cp->dns_pool); | ||
worker->cp->addr = addr; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coincidentally I just started a discussion there https://lists.apache.org/thread/jftfd6mo6p3b0tw694rlh09gdl7189hq about the unsafety of changing worker->cp->addr like this. We need a thread-safe way to do it, probably outside the scope of this PR though..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not to talk about changing worker->s->hostname_ex/port (even under the worker lock) while there are concurrent readers like here which don't take the lock.
Is it not possible to create a new worker and delete/disable the old one instead? This might be easier from a synchronisation perspective. |
Disabling the old worker is not possible because we will eventually run out of workers. I propose to use a lock to make sure that read and write operations are seen by every thread. |
Hello ylavic, |
The problem is that the worker shared data are used in multiple place during runtime, just look at worker->s->name/hostname usages, we can't change the ->name (identifier) of the worker or the ->hostname/port underneath the threads using them for current connections, and we can't (won't) synchronize everywhere neither. The only sane way I can think of to replace a proxy worker is:
(actually order between 2. and 3. is business decision, not functional) So implementing 3. is what's missing, for that we'd need to keep track of the used and unused worker shared slots to reuse them, and of the users. Not a trivial change, but probably doable. Worth it I don't know, one can also over-reserve worker slots and schedule a restart from time to time too. |
Hello @ylavic, thank you for the reply. Our need can be fullfilled by updating the url for a disabled worker only, so what we intend to do is the following: From httpd side, this means that we need to add a check that a worker is disabled before allowing to update its url, otherwise the post will return an error. Do you think we can proceed with this suggestion ? |
Unfortunately this is still racy, some requests might still arrive and reach the disabled worker, and httpd still needs a stable worker name/hostname to behave correctly (return 503) in this case. |
The goal of this pr is to change the balancer member url (host and port) dynamically.
In order to edit a member, we defined 2 params to pass:
b_ewyes: a boolean that needs to be set to 1 for the edit to work
b_ewrkr: the new url for the worker
The UI was updated to account for these 2 fields.