feat(iota-core): gRPC PoC to replace the REST-API for the checkpoint fetching of indexer #7511
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 6 Skipped Deployments
|
| // Modifications Copyright (c) 2025 IOTA Stiftung | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| //! gRPC-specific versioned types for forward compatibility. |
There was a problem hiding this comment.
These should probably be in the iota-grpc-api crate, but depends on how these evolve when we extend the API
There was a problem hiding this comment.
Agreed. Previously we put the types in the iota-gprc-api, and then we moved to the iota-types. Maybe we can decide it later depends on how we extend the API.
There was a problem hiding this comment.
I would then make it the other way around, as long as it's for internal use, let' put them in iota-grpc-api as alex said. They are only used there for now anyways, right?
bingyanglin
force-pushed
the
feat/grpc-poc
branch
4 times, most recently
from
9a7c4fa to
ca34492
Compare
bingyanglin
force-pushed
the
feat/grpc-poc
branch
from
a50cd36 to
54fe920
Compare
bingyanglin
force-pushed
the
feat/grpc-poc
branch
from
8c26881 to
ddad9c8
Compare
|
✅ Vercel Preview Deployment is ready! |
crates/iota-node/src/lib.rs
Outdated
| let rest_read_store = std::sync::Arc::new(RestReadStore::new(state.clone(), rocks)); | ||
| // Use the shared broadcast channel and buffer for gRPC checkpoint streaming | ||
| let grpc_service = | ||
| CheckpointGrpcService::new(rest_read_store, summary_tx.clone(), data_tx.clone()); | ||
| tokio::spawn(async move { | ||
| info!("Starting gRPC server on {grpc_api_address}"); | ||
| tonic::transport::Server::builder() | ||
| .add_service(CheckpointServiceServer::new(grpc_service)) | ||
| .serve(grpc_api_address) | ||
| .await | ||
| .expect("gRPC server failed"); | ||
| }); | ||
| (Some(summary_tx), Some(data_tx)) |
There was a problem hiding this comment.
We should refactor that into it's own function. I'm also not sure if a validator should run these APIs at all. So maybe better check how we build the JSON and REST server in build_http_server or start_graphiql_server and do something similar.
There was a problem hiding this comment.
Fixed it at af6baf4. Thanks!
Co-authored-by: muXxer <[email protected]>
Co-authored-by: muXxer <[email protected]>
Co-authored-by: muXxer <[email protected]>
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub grpc_api_address: Option<SocketAddr>, | ||
| /// Flag to enable the gRPC API. | ||
| #[serde(default = "bool_false")] |
There was a problem hiding this comment.
Isn't the normal default false anyways?
| @@ -405,6 +418,10 @@ pub fn bool_true() -> bool { | |||
| true | |||
| } | |||
|
|
|||
| pub fn bool_false() -> bool { | |||
There was a problem hiding this comment.
If false is the default, we can remove that function.
| @@ -1059,6 +1060,14 @@ impl TestClusterBuilder { | |||
| self | |||
| } | |||
|
|
|||
| pub fn with_fullnode_grpc_api_address(mut self, addr: SocketAddr) -> Self { | |||
There was a problem hiding this comment.
Wouldn't that overwrite the existing fullnode_grpc_api_config that might already be passed by with_fullnode_grpc_api_config? Wouldn't it be better to remove this function, and let the caller decide to pass the config with the right address and the defaults for the other values? Or do we have this pattern somewhere else in the codebase already?
| // Modifications Copyright (c) 2025 IOTA Stiftung | ||
| // SPDX-License-Identifier: Apache-2.0 | ||
|
|
||
| //! gRPC-specific versioned types for forward compatibility. |
There was a problem hiding this comment.
I would then make it the other way around, as long as it's for internal use, let' put them in iota-grpc-api as alex said. They are only used there for now anyways, right?
| } | ||
|
|
||
| pub fn with_fullnode_grpc_api_address(mut self, addr: SocketAddr) -> Self { | ||
| self.fullnode_grpc_api_config = Some(iota_grpc_api::Config { |
There was a problem hiding this comment.
same here, it would overwrite existing configs. Maybe remove the with_fullnode_grpc_api_address.
| @@ -397,6 +401,16 @@ impl FullnodeConfigBuilder { | |||
| self | |||
| } | |||
|
|
|||
| pub fn with_grpc_api_address(mut self, addr: SocketAddr) -> Self { | |||
There was a problem hiding this comment.
Let's pass the full config instead of only the address.
| let _ = data_tx.send(grpc_data); | ||
| } | ||
|
|
||
| info!( |
There was a problem hiding this comment.
Should this really be an info message? That will spam the logs.
| @@ -459,6 +503,7 @@ impl CheckpointExecutor { | |||
| .await | |||
| .expect("Failed to accumulate running root"); | |||
| self.bump_highest_executed_checkpoint(checkpoint); | |||
| self.broadcast_grpc_checkpoint(checkpoint, all_tx_digests); | |||
There was a problem hiding this comment.
Please add a comment:
// `broadcast_grpc_checkpoint` for the last checkpoint of an epoch is handled specially in `check_epoch_last_checkpoint`There was a problem hiding this comment.
Also here, maybe we already have checkpoint_data from the option above, so we could pass that here. Then we don't need to load it before broadcasting.
| @@ -736,6 +781,8 @@ impl CheckpointExecutor { | |||
|
|
|||
| self.bump_highest_executed_checkpoint(checkpoint); | |||
|
|
|||
| self.broadcast_grpc_checkpoint(checkpoint, &all_tx_digests); | |||
There was a problem hiding this comment.
IMO here you would already have the checkpoint_data, so you could pass that instead of loading it in the broadcast_grpc_checkpoint.
Description of change
Currently the indexer syncs the checkpoints via a REST-API via polling, or via filesystem.
It would be a nice proof of concept to replace those methods with a gRPC API that supports subscriptions to get new checkpoints. This should reduce the latency between checkpoint creation and indexing.
The profiling results by using a local-network (4 validator, 4 nodes, 2 indexers, 2 Postgres DBs):
More details and steps to reproduce the profiling results were documented and discussed at #6995
Links to any relevant issues
#6995
How the change has been tested
Release Notes