[Indexer-Grpc-V2] Add ConnectionManager. #15750

grao1991 · 2025-01-16T06:48:18Z

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

I have read and followed the CONTRIBUTING doc
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I identified and added all stakeholders and component owners affected by this change as reviewers
I tested both happy and unhappy path of the functionality
I have made corresponding changes to the documentation

trunk-io · 2025-01-16T06:48:22Z

⏱️ 1h 19m total CI duration on this PR

Slowest 15 Jobs	Cumulative Duration	Recent Runs
execution-performance / single-node-performance	25m	🟩 🟩
test-target-determinator	10m	🟩 🟩
check-dynamic-deps	9m	🟩 🟩 🟩 🟩
execution-performance / test-target-determinator	9m	🟩 🟩
rust-cargo-deny	6m	🟩 ⬜ 🟩 🟩
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
fetch-last-released-docker-image-tag	3m	🟩 🟩
general-lints	2m	🟩 🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩
file_change_determinator	42s	🟩 🟩 🟩 🟩
rust-lints	39s	⬜
file_change_determinator	25s	🟩 🟩
permission-check	14s	🟩 🟩 🟩 🟩
permission-check	10s	🟩 🟩

🚨 1 job on the last run was significantly faster/slower than expected

Job	Duration	vs 7d avg	Delta
execution-performance / single-node-performance	25m	15m

_{settings ⋅ feedback ⋅ docs ⋅ learn more about trunk.io}

larry-aptos · 2025-01-16T18:55:14Z

ecosystem/indexer-grpc/indexer-grpc-data-service-v2/src/connection_manager.rs

+            .collect()
+    }
+
+    fn maybe_add_sample(&mut self, version: u64, size_bytes: u64) {


does ringbuffer make this simpler since the sample size is fixed?

not really, since I'm keeping two VecDeque. (the push doesn't return the oldest element when it's full in ringbuffer crate)

larry-aptos · 2025-01-16T19:12:24Z

ecosystem/indexer-grpc/indexer-grpc-data-service-v2/src/connection_manager.rs

+    size_bytes: u64,
+}
+
+pub(crate) struct ConnectionManager {


nit: some documentation comments here. the ConnectionManager is critical in the stack based on my understanding.

also i see that String is often used here as the service address. let's either use url/socketaddr or typedef it to avoid it mixed with regular strings.

larry-aptos

take a first round of review; looks good! some nits.

taking a closer look...

github-actions · 2025-01-19T21:37:02Z

✅ Forge suite `realistic_env_max_load` success on `af3aa4c750d1d3fdbc048190f3d5bf409a07673d`

two traffics test: inner traffic : committed: 14535.39 txn/s, latency: 2733.48 ms, (p50: 2400 ms, p70: 2600, p90: 3600 ms, p99: 5700 ms), latency samples: 5526640
two traffics test : committed: 99.98 txn/s, latency: 2413.88 ms, (p50: 1400 ms, p70: 2500, p90: 5100 ms, p99: 5800 ms), latency samples: 1780
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.538, avg: 1.193", "ConsensusProposalToOrdered: max: 0.305, avg: 0.292", "ConsensusOrderedToCommit: max: 0.412, avg: 0.398", "ConsensusProposalToCommit: max: 0.702, avg: 0.689"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 1.08s no progress at version 909140 (avg 0.20s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.71s no progress at version 1924074 (avg 0.62s) [limit 16].
Test Ok

github-actions · 2025-01-19T21:37:45Z

✅ Forge suite `compat` success on `17540fad8e88ab5681f3a91190b9f5d37e53d2ef` ==> `af3aa4c750d1d3fdbc048190f3d5bf409a07673d`

Compatibility test results for 17540fad8e88ab5681f3a91190b9f5d37e53d2ef ==> af3aa4c750d1d3fdbc048190f3d5bf409a07673d (PR)
1. Check liveness of validators at old version: 17540fad8e88ab5681f3a91190b9f5d37e53d2ef
compatibility::simple-validator-upgrade::liveness-check : committed: 14425.16 txn/s, latency: 2153.63 ms, (p50: 2200 ms, p70: 2300, p90: 2500 ms, p99: 3600 ms), latency samples: 470060
2. Upgrading first Validator to new version: af3aa4c750d1d3fdbc048190f3d5bf409a07673d
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 4063.33 txn/s, latency: 7691.53 ms, (p50: 8400 ms, p70: 9100, p90: 9600 ms, p99: 9700 ms), latency samples: 89600
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 4051.55 txn/s, latency: 8417.04 ms, (p50: 9300 ms, p70: 9600, p90: 9900 ms, p99: 10300 ms), latency samples: 142760
3. Upgrading rest of first batch to new version: af3aa4c750d1d3fdbc048190f3d5bf409a07673d
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 4242.95 txn/s, latency: 7304.57 ms, (p50: 8100 ms, p70: 8700, p90: 9200 ms, p99: 9300 ms), latency samples: 91220
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4262.03 txn/s, latency: 7989.49 ms, (p50: 8900 ms, p70: 9000, p90: 9300 ms, p99: 9300 ms), latency samples: 150480
4. upgrading second batch to new version: af3aa4c750d1d3fdbc048190f3d5bf409a07673d
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 2385.11 txn/s, submitted: 2385.28 txn/s, expired: 0.18 txn/s, latency: 3720.66 ms, (p50: 4300 ms, p70: 4400, p90: 5000 ms, p99: 5100 ms), latency samples: 148269
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 8324.47 txn/s, latency: 4121.73 ms, (p50: 4500 ms, p70: 4500, p90: 4800 ms, p99: 5200 ms), latency samples: 276180
5. check swarm health
Compatibility test for 17540fad8e88ab5681f3a91190b9f5d37e53d2ef ==> af3aa4c750d1d3fdbc048190f3d5bf409a07673d passed
Test Ok

grao1991 requested review from larry-aptos and sitalkedia January 16, 2025 06:48

grao1991 marked this pull request as ready for review January 16, 2025 07:45

larry-aptos reviewed Jan 16, 2025

View reviewed changes

larry-aptos approved these changes Jan 16, 2025

View reviewed changes

grao1991 force-pushed the grao_connection_manager branch from 55719c5 to 76c1d10 Compare January 18, 2025 03:40

grao1991 enabled auto-merge (squash) January 18, 2025 03:41

This comment has been minimized.

Sign in to view

msmouse approved these changes Jan 18, 2025

View reviewed changes

[Indexer-Grpc-V2] Add ConnectionManager.

af3aa4c

grao1991 force-pushed the grao_connection_manager branch from 76c1d10 to af3aa4c Compare January 19, 2025 21:10

This comment has been minimized.

Sign in to view

grao1991 merged commit 5ad6583 into main Jan 19, 2025
46 checks passed

grao1991 deleted the grao_connection_manager branch January 19, 2025 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Indexer-Grpc-V2] Add ConnectionManager. #15750

[Indexer-Grpc-V2] Add ConnectionManager. #15750

grao1991 commented Jan 16, 2025

trunk-io bot commented Jan 16, 2025 •

edited

Loading

larry-aptos Jan 16, 2025 •

edited

Loading

grao1991 Jan 16, 2025

larry-aptos Jan 16, 2025 •

edited

Loading

larry-aptos left a comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 19, 2025

github-actions bot commented Jan 19, 2025

[Indexer-Grpc-V2] Add ConnectionManager. #15750

[Indexer-Grpc-V2] Add ConnectionManager. #15750

Conversation

grao1991 commented Jan 16, 2025

Description

How Has This Been Tested?

Key Areas to Review

Type of Change

Which Components or Systems Does This Change Impact?

Checklist

trunk-io bot commented Jan 16, 2025 • edited Loading

larry-aptos Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

grao1991 Jan 16, 2025

Choose a reason for hiding this comment

larry-aptos Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

larry-aptos left a comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

github-actions bot commented Jan 19, 2025

✅ Forge suite realistic_env_max_load success on af3aa4c750d1d3fdbc048190f3d5bf409a07673d

github-actions bot commented Jan 19, 2025

✅ Forge suite compat success on 17540fad8e88ab5681f3a91190b9f5d37e53d2ef ==> af3aa4c750d1d3fdbc048190f3d5bf409a07673d

trunk-io bot commented Jan 16, 2025 •

edited

Loading

larry-aptos Jan 16, 2025 •

edited

Loading

larry-aptos Jan 16, 2025 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `af3aa4c750d1d3fdbc048190f3d5bf409a07673d`

✅ Forge suite `compat` success on `17540fad8e88ab5681f3a91190b9f5d37e53d2ef` ==> `af3aa4c750d1d3fdbc048190f3d5bf409a07673d`