feat(sequencer, charts)!: support uds for abci #1877
Conversation
joroshiba
force-pushed
the
joroshiba/uds-for-abci
branch
4 times, most recently
from
3b54ffc to
9a1a550
Compare
joroshiba
changed the title
joroshiba
force-pushed
the
joroshiba/uds-for-abci
branch
from
55b645e to
e931f1c
Compare
joroshiba
changed the title
joroshiba
force-pushed
the
joroshiba/uds-for-abci
branch
from
e931f1c to
6a0632a
Compare
joroshiba
force-pushed
the
joroshiba/uds-for-abci
branch
from
6a0632a to
3daee25
Compare
| mempool_service: service::Mempool, | ||
| listen_url: &str, | ||
| server_exit_tx: oneshot::Sender<()>, | ||
| ) -> Result<JoinHandle<()>, Report> { |
There was a problem hiding this comment.
Replace this by the eyre::Result type alias:
| ) -> Result<JoinHandle<()>, Report> { | |
| ) -> eyre::Result<JoinHandle<()>> { |
| service::Info::new(storage.clone()).wrap_err("failed initializing info service")?; | ||
| let snapshot_service = service::Snapshot; | ||
|
|
||
| // Builds the server but does not start listening. |
| .finish() | ||
| .ok_or_eyre("server builder didn't return server; are all fields set?")?; | ||
|
|
||
| // Validate and parse the listen_url received from the config. |
There was a problem hiding this comment.
This (as well as the subsequent server start) could be done more elegantly by providing a type AbciListenUrl with variants Tcp and Unix and implement FromStr for it. Then we could also have a few simple unit tests.
| "unix" => match abci_url.to_file_path() { | ||
| Ok(_) => Ok(abci_url.path().to_string()), | ||
| Err(e) => Err(eyre!( | ||
| "failed parsing unix listen_addr file path, error: {:?}", |
There was a problem hiding this comment.
I figure eyre::WrapErr::wrap_err was not doable because this is a boxed error? We have solved a related issue in astria_eyre by wrapping the boxed error. You can then construct a proper chain:
eyre!(e).wrap_err("failed rasing unix listen_addr file path")
There was a problem hiding this comment.
I have added an enum AbciListenUrl with variants Tcp and Uds, together with a FromStr impl. I have also moved the parse to the config because that seemed to be the appropriate place for it (sequencer always runs by binding to a socket; lazy eval at a later point does not seem to make sense in this case).
Final change was from ASTRIA_SEQUENCER_ABCI_LISTENER_URL to ASTRIA_SEQUENCER_ABCI_LISTEN_URL because we are listening on that socket, not expecting a listener (CometBFT connects to the app, not the other way round).
Summary
Add support to the sequencer to support running the ABCI connection over UDS instead of TCP only, updates to chart to use the UDS connection by default.
Background
Local testing found up to a 25x speed up using UDS instead of TCP loopback for ABCI communication. We can continue to offer the option of using TCP connections, and support the faster connection within our charts by default.
Changes
listen_addrconfig with newabci_listener_urlconfig where protocol is used to inform sequencer whether to use tcp or unixTesting
Synced a full node with mainnet, and tested with ABCI replay.
Changelogs
Changelogs updated.
Breaking Changelist
ASTRIA_SEQUENCER_LISTEN_ADDRconfig variable, replaced withASTRIA_SEQUENCER_ABCI_LISTENER_URLRelated Issues
closes #1891