feat: dynamic creation and removal of workers #989
Conversation
SantiagoPittella
force-pushed
the
santiagopittella-dynamic-workers
branch
from
7b4f72b to
326589e
Compare
| This changes will be persisted to the configuration file. | ||
|
|
||
| Note that, in order to update the workers, the proxy must be running in the same computer as the command is being executed because it will check if the client address is localhost to avoid any security issues. | ||
|
|
||
| Both the worker and the proxy will use the `info` log level by default, but it can be changed by setting the `RUST_LOG` environment variable. |
There was a problem hiding this comment.
nit: I think this sentence should be in another section, as it's not related to the update of workers.
bin/tx-prover/src/proxy/mod.rs
Outdated
| if let Some(index) = available_workers.iter().position(|w| w == &worker) { | ||
| available_workers.remove(index); | ||
| } | ||
| if let Some(index) = current_workers.iter().position(|w| w == &worker) { | ||
| current_workers.remove(index); | ||
| } |
There was a problem hiding this comment.
Here I think we can use the Vec::retain method which is simpler
There was a problem hiding this comment.
Sounds good.
bin/tx-prover/src/proxy/mod.rs
Outdated
| let mut available_workers = self.available_workers.write().await; | ||
| let mut current_workers = self.current_workers.write().await; |
There was a problem hiding this comment.
The fact that we have two separate locks for the separate worker lists may lead to a deadlock (I don't think it's the case here as the lock order is always the same). We may want to unify them so that we don't have that possibility.
There was a problem hiding this comment.
I could not see a situation on our particular code that leads to a deadlock, as you said, we always lock in the same order. Though if we prefer I can unify the locks.
I have been investigating this, an attacker could impersonate "localhost" from outside of the same machine only if he has access to the same VPN that the server is connected to or he gains SSH access. I'm currently investigating a bit more on Pingora internals to check if I can create another cc @bobbinth |
SantiagoPittella
force-pushed
the
santiagopittella-dynamic-workers
branch
from
ce83e2c to
254537b
Compare
SantiagoPittella
force-pushed
the
santiagopittella-dynamic-workers
branch
from
e19984a to
05da62e
Compare
feat: dynamic creation and removal of workers fix: don't allow worker duplication, don't panic on invalid action
feat: add command to add/remove workers improve update workers response improve error handling add status for workers docs: update README
feat: use two lists instead of Worker Status fix: use proxy config from file in update-worker docs: improve code documentation docs: add entry to changelog use vec::retain update readme adding logging sections
fix: separate update workers command in two commands refactor: use only one vec to store workers chore: use X-Workers-Count, add doc for function fix: use correct header chore: rename X-Worker-Count, fix doc chore: rename current_workers, rename current_workers_count feat: implement TryInto<WorkerConfig> for &Worker
SantiagoPittella
force-pushed
the
santiagopittella-dynamic-workers
branch
from
05da62e to
e5097fe
Compare
Closes #963
Adds a command to add/remove workers from a running proxy. It updates the configuration file.
Can be used by running:
It works by keeping both a list of available workers and another for all the workers in the proxy. The proxy can be updated only if the sender of the update is in the localhost.