[Indexer-Grpc-V2] Add protos. #15701

grao1991 · 2025-01-09T22:57:33Z

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-09T22:57:36Z

⏱️ 3h 5m total CI duration on this PR

Slowest 15 Jobs	Cumulative Duration	Recent Runs
execution-performance / single-node-performance	1h 10m	🟩 🟥 🟩
rust-move-tests	13m	🟩
rust-move-tests	13m	🟩
rust-move-tests	13m	🟩
forge-compat-test / forge	13m	🟩
test-target-determinator	11m	🟩 🟩 🟩
execution-performance / test-target-determinator	11m	🟩 🟩 🟩
rust-doc-tests	7m	🟩
rust-cargo-deny	7m	🟩 🟩 🟩 🟩
check-dynamic-deps	6m	🟩 🟩 🟩 🟩
fetch-last-released-docker-image-tag	6m	🟩 🟩 🟩
rust-doc-tests	5m	🟩
rust-doc-tests	5m	🟩
general-lints	2m	🟩 🟩 🟩 🟩
semgrep/ci	2m	🟩 🟩 🟩 🟩

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

larry-aptos · 2025-01-10T22:00:38Z

protos/proto/aptos/indexer/v1/grpc.proto

+import "aptos/transaction/v1/transaction.proto";
+import "aptos/util/timestamp/timestamp.proto";
+
+message StreamProgressSampleProto {


i feel Proto here is a little redundant; either remove or use unit something?

I use here because in the rust code I have a corresponding struct called StreamProgressSample. Just want to have a different name here.

larry-aptos · 2025-01-10T22:09:16Z

protos/proto/aptos/indexer/v1/grpc.proto

+}
+
+message StreamProgress {
+  repeated StreamProgressSampleProto samples = 1;


What's the intent behind collecting multiple samples here?

calculate tps, etc.

sync'ed offline; for monitoring purpose.

larry-aptos · 2025-01-10T22:17:42Z

protos/proto/aptos/indexer/v1/grpc.proto

+  optional StreamInfo stream_info = 3;
+}
+
+message FullnodeInfo {


Do you think we should include validation here? like chain id?

Good point. Done.

larry-aptos · 2025-01-10T22:18:18Z

overall looks good; some nits/questions.

larry-aptos

Need a second pair of eyes 👀 overall looks good

larry-aptos · 2025-01-10T22:20:10Z

also i see that the master address in the message; does election process leverage the proposed protos or you'll introduce new grpcs?

grao1991 · 2025-01-10T22:22:05Z

also i see that the master address in the message; does election process leverage the proposed protos or you'll introduce new grpcs?

It won't rely on this. (and based on the design review I will probably not implement it in the beginning, just configure one of them through config).

larry-aptos · 2025-01-13T20:06:04Z

you may need rebase

grao1991 · 2025-01-13T20:20:43Z

you may need rebase

Done

graphite-app · 2025-01-13T20:40:37Z

ecosystem/indexer-grpc/indexer-grpc-fullnode/src/fullnode_data_service.rs

+    async fn ping(
+        &self,
+        _request: Request<PingFullnodeRequest>,
+    ) -> Result<Response<PingFullnodeResponse>, Status> {
+        unimplemented!()
+    }


Using unimplemented!() will cause a panic at runtime. Consider returning a proper gRPC response instead:

Ok(Response::new(PingFullnodeResponse { info: None }))

This maintains the gRPC contract while indicating the endpoint is not yet implemented.

Spotted by Graphite Reviewer

Is this helpful? React 👍 or 👎 to let us know.

github-actions · 2025-01-13T20:43:33Z

✅ Forge suite `realistic_env_max_load` success on `6580930bd04073ed7d6fb3835d6b870d63076dcb`

two traffics test: inner traffic : committed: 14891.00 txn/s, latency: 2668.37 ms, (p50: 2700 ms, p70: 2700, p90: 3000 ms, p99: 3300 ms), latency samples: 5661880
two traffics test : committed: 99.96 txn/s, latency: 1432.57 ms, (p50: 1300 ms, p70: 1400, p90: 1500 ms, p99: 3100 ms), latency samples: 1640
Latency breakdown for phase 0: ["MempoolToBlockCreation: max: 1.524, avg: 1.399", "ConsensusProposalToOrdered: max: 0.288, avg: 0.287", "ConsensusOrderedToCommit: max: 0.301, avg: 0.291", "ConsensusProposalToCommit: max: 0.587, avg: 0.578"]
Max non-epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.74s no progress at version 29148 (avg 0.19s) [limit 15].
Max epoch-change gap was: 0 rounds at version 0 (avg 0.00) [limit 4], 0.54s no progress at version 2505965 (avg 0.54s) [limit 16].
Test Ok

github-actions · 2025-01-13T22:41:15Z

✅ Forge suite `compat` success on `6593fb81261f25490ffddc2252a861c994234c2a` ==> `6580930bd04073ed7d6fb3835d6b870d63076dcb`

Compatibility test results for 6593fb81261f25490ffddc2252a861c994234c2a ==> 6580930bd04073ed7d6fb3835d6b870d63076dcb (PR)
1. Check liveness of validators at old version: 6593fb81261f25490ffddc2252a861c994234c2a
compatibility::simple-validator-upgrade::liveness-check : committed: 18225.39 txn/s, latency: 1918.90 ms, (p50: 2000 ms, p70: 2100, p90: 2200 ms, p99: 2200 ms), latency samples: 586080
2. Upgrading first Validator to new version: 6580930bd04073ed7d6fb3835d6b870d63076dcb
compatibility::simple-validator-upgrade::single-validator-upgrading : committed: 7506.78 txn/s, latency: 3995.27 ms, (p50: 4600 ms, p70: 4900, p90: 5200 ms, p99: 5300 ms), latency samples: 135520
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 7408.41 txn/s, latency: 4555.31 ms, (p50: 4900 ms, p70: 5100, p90: 5200 ms, p99: 5400 ms), latency samples: 248740
3. Upgrading rest of first batch to new version: 6580930bd04073ed7d6fb3835d6b870d63076dcb
compatibility::simple-validator-upgrade::half-validator-upgrading : committed: 7665.28 txn/s, latency: 3928.93 ms, (p50: 4500 ms, p70: 4700, p90: 4900 ms, p99: 4900 ms), latency samples: 143680
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 7313.15 txn/s, latency: 4499.40 ms, (p50: 4900 ms, p70: 4900, p90: 5000 ms, p99: 5100 ms), latency samples: 252500
4. upgrading second batch to new version: 6580930bd04073ed7d6fb3835d6b870d63076dcb
compatibility::simple-validator-upgrade::rest-validator-upgrading : committed: 13316.38 txn/s, latency: 2174.24 ms, (p50: 2400 ms, p70: 2500, p90: 2900 ms, p99: 2900 ms), latency samples: 228380
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 13369.66 txn/s, latency: 2435.70 ms, (p50: 2400 ms, p70: 2800, p90: 2900 ms, p99: 3000 ms), latency samples: 430860
5. check swarm health
Compatibility test for 6593fb81261f25490ffddc2252a861c994234c2a ==> 6580930bd04073ed7d6fb3835d6b870d63076dcb passed
Test Ok

grao1991 requested review from areshand, geekflyer, msmouse, banool, sitalkedia, bowenyang007 and larry-aptos January 9, 2025 22:57

grao1991 enabled auto-merge (squash) January 9, 2025 23:27

This comment has been minimized.

Sign in to view

larry-aptos reviewed Jan 10, 2025

View reviewed changes

grao1991 requested a review from larry-aptos January 10, 2025 22:07

larry-aptos reviewed Jan 10, 2025

View reviewed changes

larry-aptos approved these changes Jan 10, 2025

View reviewed changes

grao1991 force-pushed the grao_proto branch from 081e873 to 36f0662 Compare January 10, 2025 22:27

This comment has been minimized.

Sign in to view

[Indexer-Grpc-V2] Add protos.

6580930

grao1991 force-pushed the grao_proto branch from 36f0662 to 6580930 Compare January 13, 2025 20:15

This comment has been minimized.

Sign in to view

graphite-app bot reviewed Jan 13, 2025

View reviewed changes

sitalkedia approved these changes Jan 13, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

grao1991 merged commit 8f3d204 into main Jan 13, 2025
43 of 46 checks passed

grao1991 deleted the grao_proto branch January 13, 2025 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Indexer-Grpc-V2] Add protos. #15701

[Indexer-Grpc-V2] Add protos. #15701

grao1991 commented Jan 9, 2025

trunk-io bot commented Jan 9, 2025 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

larry-aptos Jan 10, 2025

grao1991 Jan 10, 2025

larry-aptos Jan 10, 2025

grao1991 Jan 10, 2025

larry-aptos Jan 10, 2025

larry-aptos Jan 10, 2025

grao1991 Jan 10, 2025

larry-aptos commented Jan 10, 2025

larry-aptos left a comment

larry-aptos commented Jan 10, 2025

grao1991 commented Jan 10, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

larry-aptos commented Jan 13, 2025

grao1991 commented Jan 13, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

graphite-app bot Jan 13, 2025

github-actions bot commented Jan 13, 2025

This comment has been minimized.

github-actions bot commented Jan 13, 2025

[Indexer-Grpc-V2] Add protos. #15701

[Indexer-Grpc-V2] Add protos. #15701

Conversation

grao1991 commented Jan 9, 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 9, 2025 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

larry-aptos Jan 10, 2025

Choose a reason for hiding this comment

grao1991 Jan 10, 2025

Choose a reason for hiding this comment

larry-aptos Jan 10, 2025

Choose a reason for hiding this comment

grao1991 Jan 10, 2025

Choose a reason for hiding this comment

larry-aptos Jan 10, 2025

Choose a reason for hiding this comment

larry-aptos Jan 10, 2025

Choose a reason for hiding this comment

grao1991 Jan 10, 2025

Choose a reason for hiding this comment

larry-aptos commented Jan 10, 2025

larry-aptos left a comment

Choose a reason for hiding this comment

larry-aptos commented Jan 10, 2025

grao1991 commented Jan 10, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

larry-aptos commented Jan 13, 2025

grao1991 commented Jan 13, 2025

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

graphite-app bot Jan 13, 2025

Choose a reason for hiding this comment

github-actions bot commented Jan 13, 2025

✅ Forge suite realistic_env_max_load success on 6580930bd04073ed7d6fb3835d6b870d63076dcb

This comment has been minimized.

github-actions bot commented Jan 13, 2025

✅ Forge suite compat success on 6593fb81261f25490ffddc2252a861c994234c2a ==> 6580930bd04073ed7d6fb3835d6b870d63076dcb

trunk-io bot commented Jan 9, 2025 •

edited

Loading

✅ Forge suite `realistic_env_max_load` success on `6580930bd04073ed7d6fb3835d6b870d63076dcb`

✅ Forge suite `compat` success on `6593fb81261f25490ffddc2252a861c994234c2a` ==> `6580930bd04073ed7d6fb3835d6b870d63076dcb`