Skip to content

Add support for batched requests/responses in JSON RPC#3280

Closed
thanethomson wants to merge 5 commits intotendermint:developfrom
thanethomson:issues/3213
Closed

Add support for batched requests/responses in JSON RPC#3280
thanethomson wants to merge 5 commits intotendermint:developfrom
thanethomson:issues/3213

Conversation

@thanethomson
Copy link
Contributor

@thanethomson thanethomson commented Feb 8, 2019

As per #3213, this PR will add support for batched JSON RPC requests/responses.

The suggestion here is to alter the HTTP RPC client interface to have a batching mode, which allows for the same calls as previously, but instead of sending the requests immediately, the JSONRPCClient will buffer the RPCRequest objects until c.SendBatch() is called:

c := client.NewHTTP(rpcAddr, "/websocket")
c.StartBatch()
_, err := c.BroadcastTxCommit(tx1)
_, err = c.BroadcastTxCommit(tx2)
// nothing sent yet
results, err := c.SendBatch()
// batch is now sent, and one can iterate through `results` here...
// ...
// this request is now sent immediately, since we're no longer in batch mode
res, err := c.BroadcastTxCommit(tx3)

There are a few more things I'd like to test before I'd consider this finished though - will push those tests shortly.

  • Updated all relevant documentation in docs
  • Updated all code comments where relevant
  • Wrote tests
  • Updated CHANGELOG_PENDING.md

As per #3213, this adds support for [JSON RPC batch requests and
responses](https://www.jsonrpc.org/specification#batch).
@codecov-io
Copy link

codecov-io commented Feb 8, 2019

Codecov Report

Merging #3280 into develop will increase coverage by 0.07%.
The diff coverage is 94.44%.

@@             Coverage Diff             @@
##           develop    #3280      +/-   ##
===========================================
+ Coverage     63.5%   63.58%   +0.07%     
===========================================
  Files          215      215              
  Lines        17782    17799      +17     
===========================================
+ Hits         11293    11317      +24     
+ Misses        5550     5544       -6     
+ Partials       939      938       -1
Impacted Files Coverage Δ
rpc/client/httpclient.go 70.94% <94.44%> (+1.35%) ⬆️
libs/events/events.go 93.2% <0%> (-4.86%) ⬇️
p2p/pex/pex_reactor.go 79.55% <0%> (+0.31%) ⬆️
consensus/state.go 79% <0%> (+0.47%) ⬆️
consensus/reactor.go 71.66% <0%> (+1.06%) ⬆️

@thanethomson thanethomson changed the title [WIP] Add support for batched requests/responses in JSON RPC Add support for batched requests/responses in JSON RPC Feb 8, 2019
@thanethomson
Copy link
Contributor Author

I'm closing this here and will re-open in a separate PR from a branch within the Tendermint repo (this original PR came from my own fork of Tendermint).

melekes pushed a commit that referenced this pull request Apr 17, 2019
Continues from #3280 in building support for batched requests/responses in the JSON RPC (as per issue #3213).

* Add JSON RPC batching for client and server

As per #3213, this adds support for [JSON RPC batch requests and
responses](https://www.jsonrpc.org/specification#batch).

* Add additional checks to ensure client responses are the same as results

* Fix case where a notification is sent and no response is expected

* Add test to check that JSON RPC notifications in a batch are left out in responses

* Update CHANGELOG_PENDING.md

* Update PR number now that PR has been created

* Make errors start with lowercase letter

* Refactor batch functionality to be standalone

This refactors the batching functionality to rather act in a standalone
way. In light of supporting concurrent goroutines making use of the same
client, it would make sense to have batching functionality where one
could create a batch of requests per goroutine and send that batch
without interfering with a batch from another goroutine.

* Add examples for simple and batch HTTP client usage

* Check errors from writer and remove nolinter directives

* Make error strings start with lowercase letter

* Refactor examples to make them testable

* Use safer deferred shutdown for example Tendermint test node

* Recompose rpcClient interface from pre-existing interface components

* Rename WaitGroup for brevity

* Replace empty ID string with request ID

* Remove extraneous test case

* Convert first letter of errors.Wrap() messages to lowercase

* Remove extraneous function parameter

* Make variable declaration terse

* Reorder WaitGroup.Done call to help prevent race conditions in the face of failure

* Swap mutex to value representation and remove initialization

* Restore empty JSONRPC string ID in response to prevent nil

* Make JSONRPCBufferedRequest private

* Revert PR hard link in CHANGELOG_PENDING

* Add client ID for JSONRPCClient

This adds code to automatically generate a randomized client ID for the
JSONRPCClient, and adds a check of the IDs in the responses (if one was
set in the requests).

* Extract response ID validation into separate function

* Remove extraneous comments

* Reorder fields to indicate clearly which are protected by the mutex

* Refactor for loop to remove indexing

* Restructure and combine loop

* Flatten conditional block for better readability

* Make multi-variable declaration slightly more readable

* Change for loop style

* Compress error check statements

* Make function description more generic to show that we support different protocols

* Preallocate memory for request and result objects
brapse pushed a commit to brapse/tendermint that referenced this pull request Jun 5, 2019
Continues from tendermint#3280 in building support for batched requests/responses in the JSON RPC (as per issue tendermint#3213).

* Add JSON RPC batching for client and server

As per tendermint#3213, this adds support for [JSON RPC batch requests and
responses](https://www.jsonrpc.org/specification#batch).

* Add additional checks to ensure client responses are the same as results

* Fix case where a notification is sent and no response is expected

* Add test to check that JSON RPC notifications in a batch are left out in responses

* Update CHANGELOG_PENDING.md

* Update PR number now that PR has been created

* Make errors start with lowercase letter

* Refactor batch functionality to be standalone

This refactors the batching functionality to rather act in a standalone
way. In light of supporting concurrent goroutines making use of the same
client, it would make sense to have batching functionality where one
could create a batch of requests per goroutine and send that batch
without interfering with a batch from another goroutine.

* Add examples for simple and batch HTTP client usage

* Check errors from writer and remove nolinter directives

* Make error strings start with lowercase letter

* Refactor examples to make them testable

* Use safer deferred shutdown for example Tendermint test node

* Recompose rpcClient interface from pre-existing interface components

* Rename WaitGroup for brevity

* Replace empty ID string with request ID

* Remove extraneous test case

* Convert first letter of errors.Wrap() messages to lowercase

* Remove extraneous function parameter

* Make variable declaration terse

* Reorder WaitGroup.Done call to help prevent race conditions in the face of failure

* Swap mutex to value representation and remove initialization

* Restore empty JSONRPC string ID in response to prevent nil

* Make JSONRPCBufferedRequest private

* Revert PR hard link in CHANGELOG_PENDING

* Add client ID for JSONRPCClient

This adds code to automatically generate a randomized client ID for the
JSONRPCClient, and adds a check of the IDs in the responses (if one was
set in the requests).

* Extract response ID validation into separate function

* Remove extraneous comments

* Reorder fields to indicate clearly which are protected by the mutex

* Refactor for loop to remove indexing

* Restructure and combine loop

* Flatten conditional block for better readability

* Make multi-variable declaration slightly more readable

* Change for loop style

* Compress error check statements

* Make function description more generic to show that we support different protocols

* Preallocate memory for request and result objects
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants