Adding configurabele TCP buffers and minor hafixes #4649

Markuze · 2022-09-29T19:20:35Z

[Network] set socket RX/TX Buffers

Adding configurable TCP buffers for inbound and outbound TCP connections.
This change will improve networking performance, especially for long RTT connections.

Description

Test Plan

This change is

JoshLind

LGTM. Thanks @Markuze

JoshLind · 2022-10-11T14:29:26Z

config/src/config/network_config.rs

@@ -51,6 +51,10 @@ pub const MAX_MESSAGE_SIZE: usize = 64 * 1024 * 1024; /* 64 MiB */
 pub const CONNECTION_BACKOFF_BASE: u64 = 2;
 pub const IP_BYTE_BUCKET_RATE: usize = 102400 /* 100 KiB */;
 pub const IP_BYTE_BUCKET_SIZE: usize = IP_BYTE_BUCKET_RATE;
+pub const INBOUND_TCP_RX_BUFFER_SIZE: u32 = 3 * 1024 * 1024; // 3MB ~6MB/s with 500ms latency
+pub const INBOUND_TCP_TX_BUFFER_SIZE: u32 = 512 * 1024; // 1MB use a bigger spoon


Is this a technical term, or just informal wording? use a bigger spoon 😄 Not sure how to respond based on the comment lol

Informal but descriptive.

github-actions · 2022-10-11T21:48:01Z

✅ Forge suite `compat` success on `843b204dce971d98449b82624f4f684c7a18b991` ==> `495e74fa3a8357ec294b7063ae6013e51fe0c361`

Compatibility test results for 843b204dce971d98449b82624f4f684c7a18b991 ==> 495e74fa3a8357ec294b7063ae6013e51fe0c361 (PR)
1. Check liveness of validators at old version: 843b204dce971d98449b82624f4f684c7a18b991
compatibility::simple-validator-upgrade::liveness-check : 7872 TPS, 5043 ms latency, 7300 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 495e74fa3a8357ec294b7063ae6013e51fe0c361
compatibility::simple-validator-upgrade::single-validator-upgrade : 4596 TPS, 8645 ms latency, 13600 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 495e74fa3a8357ec294b7063ae6013e51fe0c361
compatibility::simple-validator-upgrade::half-validator-upgrade : 4880 TPS, 8529 ms latency, 10900 ms p99 latency,no expired txns
4. upgrading second batch to new version: 495e74fa3a8357ec294b7063ae6013e51fe0c361
compatibility::simple-validator-upgrade::rest-validator-upgrade : 7186 TPS, 5235 ms latency, 10300 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for 843b204dce971d98449b82624f4f684c7a18b991 ==> 495e74fa3a8357ec294b7063ae6013e51fe0c361 passed
Test Ok

Grafana dashboard
Humio Logs
Test runner output
Test run is land-blocking

github-actions · 2022-10-11T21:48:10Z

✅ Forge suite `land_blocking` success on `495e74fa3a8357ec294b7063ae6013e51fe0c361`

performance benchmark with full nodes : 6803 TPS, 5850 ms latency, 8700 ms p99 latency,(!) expired 360 out of 2905460 txns
Test Ok

Grafana dashboard
Humio Logs
Test runner output
Test run is land-blocking

perryjrandall · 2022-10-11T22:47:44Z

I merged this forcibly because the smoke tests were broken in CI, the test passed locally

        PASS [  65.698s] smoke-test aptos_cli::validator::test_join_and_leave_validator
------------
     Summary [  65.699s] 1 tests run: 1 passed (1 slow), 64 skipped
perry@Perrys-MBP aptos-node % LOCAL_SWARM_NODE_RELEASE=1 cargo nextest run --release --profile ci --package smoke-test --test-threads 6 --retries 3 -- aptos_cli::validator::test_join_and_leave_validator

bchocho · 2022-10-11T23:55:37Z

network/netcore/src/transport/tcp.rs

-        listener.set_nonblocking(true)?;
-        let listener = TcpListener::try_from(listener)?;
+        let addr = SocketAddr::new(ipaddr, port);
+


Did we change this to not set_nonblocking?

) ### Description This was done within `TcpListener.bind()` before #4649.

### Description Tests show that overriding the socket options for send and recv buffer actually hurts performance (even with default linux kernel settings). Logically, this rolls back #4649 but keeping the configs and code around in case this or other socket options are revisited. ### Test Plan New results: * `net_bench_two_region_env`: 800 KB/s -> 4 MB/s * `pfn_performance_with_realistic_env`: 4K -> 6K TPS

Tests show that overriding the socket options for send and recv buffer actually hurts performance (even with default linux kernel settings). Logically, this rolls back #4649 but keeping the configs and code around in case this or other socket options are revisited. New results: * `net_bench_two_region_env`: 800 KB/s -> 4 MB/s * `pfn_performance_with_realistic_env`: 4K -> 6K TPS

…0775) Tests show that overriding the socket options for send and recv buffer actually hurts performance (even with default linux kernel settings). Logically, this rolls back #4649 but keeping the configs and code around in case this or other socket options are revisited. New results: * `net_bench_two_region_env`: 800 KB/s -> 4 MB/s * `pfn_performance_with_realistic_env`: 4K -> 6K TPS

Markuze requested a review from a team as a code owner September 29, 2022 19:20

Markuze force-pushed the markuze/tcp_cfg branch from d73ca5b to 3fdc778 Compare September 29, 2022 19:24

Markuze requested review from zekun000, grao1991 and igor-aptos and removed request for a team September 29, 2022 19:42

Markuze added the CICD:run-e2e-tests when this label is present github actions will run all land-blocking e2e tests from the PR label Sep 29, 2022

Markuze requested review from sitalkedia and bchocho September 29, 2022 19:43

Markuze force-pushed the markuze/tcp_cfg branch from 3fdc778 to 695cb39 Compare September 29, 2022 19:50