0xMiden / 0xMiden/node

Proposal: gRPC streaming API

未关闭
#2,356 3 条评论 2 个 reaction 已指派 1 人 已被 @kkovaacs 认领 在 GitHub 查看
rpc
主要语言
Rust
星标
104
派生
138
平均合并
1 天 13 小时
30 天内合并 PR
56

描述

We should strongly consider replacing our request/response `SyncXxx` methods with streaming versions instead.

This has a few benefits and imo matches the current client sync process much better. As a short overview, the client syncs by:

1. Using `SyncMmr` to a specific block.
2. Using the other `SyncXxx` methods to sync up to the block from (1).

I've added @kkovaacs's AI summaries of the status quo and streaming changes. imo they make a pretty strong case for this proposal but also contains additional suggestions which are maybe less relevant. I'll add my own thoughts below.

[status quo.md](https://github.com/user-attachments/files/30185219/status.quo.md)
[streaming.md](https://github.com/user-attachments/files/30185220/streaming.md)

### Rollout

I suggest we roll this out as a non-breaking change for v0.16 by adding new gRPC streaming methods, either as a separate `rpc.Streaming` service, or simply by `XxxStreaming`.

The client can then implement and test these whenever they have time. This also means we can roll these out a method at a time without worrying about deploying updates.

If things go well, in v0.17 we yank the original methods.

### Problems this addresses

#### Response sizes

We have several issues/bugs related to this, but effectively:

Response sizes are difficult to estimate accurately since we need the item's size in protobuf plus protobufs framing thereof. And we can't have that because the response is a struct until `tonic` encodes it so we never truly have access to the response size until after we are done constructing it.

This is technically possible, but would require re-implementing grpc's codec or at least figuring it out for our collection types.

#### Pagination

Response size limits means we need to paginate user requests. Our pagination uses blocks as boundaries which complicates implementations since we must first reach the size limit, then backtrack any incomplete block.

It also means we cannot easily elide redundant data, e.g. we send every account storage update since we cannot know when the response limit may be reached. We could use an alternative pagination strategy here, but it would be convenient to have a more standardize approach.

With streaming we don't need to paginate and can instead stream the _final result_ at the request's endpoint. This addresses several issues regarding sending redundant data.

#### Complexity

imo streaming results in much simpler client side code since pagination is a fairly involved and painful process.

On the node side, we sidestep the entire problem of response sizes, to which we _do not_ have a good solution.

### Implementation

I think we should still paginate in the stream handler in the node. This avoids memory spikes and hogging the database connection. Handler becomes:

1. Load page
2. Stream page
3. Repeat

This should be standardized with a trait e.g. `async fn Paginator::load_next_page()`. Then `SyncStream` handles the orchestration.

A good follow-up optimisation is using a double-buffer for page loading so there is no downtime where we wait for the next page.

We may run into issues if this requires holding a long-lasting RocksDb snapshots but if that's the case, then its a problem with normal pagination as well.

I would also avoid a continuation token unless absolutely necessary. That means data from an interrupted stream should be considered broken and dropped. If this is deemed risky, then I would prefer the client shortens its request range to limit the database i.e. the client can "paginate".

A stream continuation token would require an absolute ordering which is not always trivially present.

### Final thoughts

Imo the only downside is the API churn this late in the game. However I do think its well worth it (if the design survives contact with reality).

贡献指南

打开贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。