Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rust RPC client does not support batching #25623

Closed
gitmalong opened this issue May 28, 2022 · 13 comments
Closed

Rust RPC client does not support batching #25623

gitmalong opened this issue May 28, 2022 · 13 comments
Labels
RPC Issues related to validator RPC rust Pull requests that update Rust code

Comments

@gitmalong
Copy link

gitmalong commented May 28, 2022

Problem

https://docs.solana.com/de/developing/clients/jsonrpc-api states Requests can be sent in batches by sending an array of JSON-RPC request objects as the data for a single POST.

Cannot see any methods within the Rust RPC Client to build and send requests as batch.

https://docs.rs/solana-client/1.7.10/solana_client/rpc_client/struct.RpcClient.html

Proposed Solution

Introduce a builder pattern to build requests for all RPC endpoints that only build but not send requests. To be downwards compatible current methods can make use of the builder pattern and then send data immediately. To allow batching users would build multiple requests and send them via send_batch.

@gitmalong
Copy link
Author

Batching could be further enhanced similar to the web3 js implementation by introducing an interval in which the client sends out all queued requests as batch.

@steveluscher
Copy link
Contributor

Somewhat related. #23627.

@steveluscher
Copy link
Contributor

Can you help me out by coming up with some concrete use cases and example code? I tried to sketch some out in this comment and it just broke my brain. There are so many edge cases and opportunities for confusion. I really crave a straightforward example that makes me go ‘yes, absolutely, we need that.’

@steveluscher steveluscher added the javascript Pull requests that update Javascript code label Jun 1, 2022
@steveluscher
Copy link
Contributor

I gave this a good think, and wrote down my thoughts here: https://twitter.com/steveluscher/status/1532443998488363008

I actually think that JSON-RPC 2.0 batching is a huge footgun, and might actually lead to worse application performance overall. Read the thread above and let me know what you think!

@gitmalong
Copy link
Author

gitmalong commented Jun 5, 2022

Hi @steveluscher !

Just to clarify cause you added the javascript label. I'm talking about a missing feature of the Rust RPC client.

Thanks for your write up on Twitter - all valid points.

Proposal to solve "Harder to process results"

Harder to route errors to your app when some inner requests fail and some succeed
Have to write request coalescing logic in your app

Processing results becomes definetely more complex but we can shift that work to the client implementation rather than to client users:

For each request that is added to the batch, user receives a future (promise in Rust) that resolves once the response arrived. That way we still need to wait until the slowest request finishes (as we still just get a single response) but there will be no need to map individual request-responds back to the application flow. Ideal for async applications.

That approach further allows us to implement an auto-batch feature which batches all requests that were added to a queue within a fixed interval. In case users want more fine-grained control they can simply create to-be-sent requests and send_batch them whenever they want.

Wondering if such a mechanism is not an existing feature of common RPC clients?

Use cases

In general I think that a tailored RPC client should support the features of the server. Otherwise the feature can be stripped from there. But it makes very much sense to be transparent and guide users about the down-sides of batching.

What you save in network overhead could get immediately lost, as fast requests in your app are made to wait on the very slowest one. Time that your app could spend rendering, is now spent waiting.

So that point is totally valid for typical web apps but might be different for backend apps or other sort of web apps.

Concrete I am working on an Rust backend app that traverses all transactions of a certain wallet (in order to retrieve a proper CSV for filing taxes. Surprisingly a huge pain within the eco system). Thereby I'm sending hundreds of gettransaction requests. As RPC nodes are typically rate limited batching makes very much sense. Not sure how much work it is on server side but I can imagine that sending hundred of gettransaction via batch is way faster than single requests.

For other uses cases we might want to reach out to the people who wrote about batching in the RPC API doc :))

Thanks for listening.

Offtopic: As you might be engaged in the Solana foundation could you raise the inability of retrieving historic Serum DEX orders towards the foundation? I would appreciate that soo much. Otherwise users have a very hard time to fulfill legal obligations for a proper tax report.

@steveluscher
Copy link
Contributor

As RPC nodes are typically rate limited batching makes very much sense.

A-ha! I think you've nailed it. When you shake apart all of the arguments for batching RPC requests, I think that this is the root of most of them. Here's the bad news though: if the purpose of a rate limit is to limit how much of an RPC's resources you can consume, then 5 getTransaction requests should count the same toward your rate limit as 1 batch request containing 5 getTransaction requests. I'm going to follow up with an RPC provider about that to see if we can get that done (sorry!). If we can get that fixed to be more equitable, maybe we can get the rate limits raised overall.

As you might be engaged in the Solana foundation could you raise the inability of retrieving historic Serum DEX orders towards the foundation?

I don't even know who I'd ask about that.

@steveluscher steveluscher changed the title RPC client does not support batching Rust RPC client does not support batching Jun 6, 2022
@gitmalong
Copy link
Author

Triton (https://rpcpool.com) counts requests by http-request but I think they can see the actual RPC requests as well. @linuskendall may want to elaborate on that. For me it is more about performance. Assuming that getTransaction is a RPC wise cheap call it takes ages to retrieve thousands of transactions when each call requires a http-request.

@steveluscher
Copy link
Contributor

Cool cool. You should 100% ask the Rust folks here for the ability to inject a custom network layer. This is our recommendation on the web3.js side for achieving batching behavior.

@kevinheavey
Copy link
Contributor

kevinheavey commented Jun 7, 2022

In my opinion, having tools to build my own JSON request body is simpler than injecting a custom network layer. Especially in Rust

@steveluscher
Copy link
Contributor

Totally! That’s absolutely what I’m suggesting. On the JS side what I’m calling a ‘network layer’ is something that wraps the browser’s fetch API. Over there we’d simply coalesce incoming requests, bundle them into a batch during a time window (build our own JavaScript request body, as you’ve said), then forward that on to the browser’s fetch API.

@steveluscher steveluscher added rust Pull requests that update Rust code RPC Issues related to validator RPC and removed javascript Pull requests that update Javascript code labels Dec 2, 2022
@xoac
Copy link
Contributor

xoac commented Aug 23, 2023

@steveluscher so I agree with json-rpc batch problem you described on twitter. But we are talking here about adding batch request support to RpcClient. This can be hidden ability when needed and doesn't need to be general function like send_batch.

For example Solana RPC json API provide getSignaturesForAddress with single API call but to get information about all this transactions you need to execute separate http request to node and you are interested in response when all of them are available.

So RpcClient could provide get_transaction_batched request that will query for all this signatures using single http-request and both client and server will benefit from this.

Building general send_batch function in rust that can also do de-serialize to some T would be also very though task. I was only able to build general one that return serde_json::Value and later use it in get_transaction_batched

#[async_trait]
impl RpcSender for HttpSender {
    fn get_transport_stats(&self) -> RpcTransportStats {
        self.stats.read().unwrap().clone()
    }

    async fn send(
        &self,
        request: RpcRequest,
        params: serde_json::Value,
    ) -> Result<serde_json::Value> {
        let request_id = self.request_id.fetch_add(1, Ordering::Relaxed);
        let request_json = request.build_request_json(request_id, params);

        let response = self.post_json(request_json).await?;

        let json = response.json::<serde_json::Value>().await?;

        parse_single_response(json).map_err(|err| err.into())
    }

    async fn send_batch(
        &self,
        reqs: Vec<(RpcRequest, serde_json::Value)>,
    ) -> Result<Vec<StdResult<serde_json::Value, RpcError>>> {
        let batch_req = reqs
            .into_iter()
            .map(|(req, params)| {
                let request_id = self.request_id.fetch_add(1, Ordering::Relaxed);
                req.build_request_json(request_id, params)
            })
            .collect::<Vec<_>>();

        let response = self.post_json(Value::Array(batch_req)).await?;

        let json = response.json::<serde_json::Value>().await?;

        Ok(parse_batch_response(json))
    }

    fn url(&self) -> String {
        self.url.clone()
    }
}

so in the RpcClient this could only batch the same type of requests or decode them to some enum

    pub async fn send_batch<T>(&self, requests: Vec<(RpcRequest, Value)>) -> ClientResult<Vec<T>>
    where
        T: serde::de::DeserializeOwned,
    {
        let response = self.sender.send_batch(requests).await?;

        let resp = response
            .into_iter()
            .map(|res| res.map_err(ClientError::from))
            .collect::<Result<Vec<_>, _>>()?;

        Ok(resp
            .into_iter()
            .map(|response| serde_json::from_value(response).unwrap())
            .collect())
    }
    
    pub async fn get_transactions_batch(
        &self,
        signatures: &[Signature],
        encoding: UiTransactionEncoding,
    ) -> ClientResult<Vec<EncodedConfirmedTransactionWithStatusMeta>> {
        let request = self.maybe_map_request(RpcRequest::GetTransaction).await?;

        let requests = signatures
            .iter()
            .map(|signature| (request, json!([signature.to_string(), encoding])))
            .collect();

        self.send_batch(requests).await
    }

It's hard to extend current RpcClient with batch ability without breaking changes but maybe would be good to create a RpcBatchClient that provide some useful batch methods.

@linuskendall
Copy link

linuskendall commented Aug 23, 2023 via email

@steveluscher
Copy link
Contributor

HTTP/2 is the way. This issue should be closed in favor of a new issue tracking implementing HTTP/2 in the Rust client. cc/ @CriesofCarrots?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Issues related to validator RPC rust Pull requests that update Rust code
Projects
None yet
Development

No branches or pull requests

6 participants