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

rpc: create buffered subscriptions on /subscribe #4521

Merged
merged 6 commits into from Mar 6, 2020

Conversation

melekes
Copy link
Contributor

@melekes melekes commented Mar 3, 2020

buffer size: 100

Closes #3935


For contributor use:

  • Wrote tests tested manually
  • Updated CHANGELOG_PENDING.md
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments
  • Re-reviewed Files changed in the Github PR explorer

@melekes melekes self-assigned this Mar 3, 2020
rpc/core/events.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Mar 5, 2020

Codecov Report

Merging #4521 into master will increase coverage by 0.07%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #4521      +/-   ##
==========================================
+ Coverage   65.22%   65.29%   +0.07%     
==========================================
  Files         229      229              
  Lines       20269    20273       +4     
==========================================
+ Hits        13220    13238      +18     
+ Misses       5994     5980      -14     
  Partials     1055     1055
Impacted Files Coverage Δ
rpc/core/events.go 0% <0%> (ø) ⬆️
libs/events/events.go 93.2% <0%> (-4.86%) ⬇️
consensus/replay.go 71.76% <0%> (-0.79%) ⬇️
privval/messages.go 100% <0%> (ø) ⬆️
consensus/state.go 77.68% <0%> (+0.51%) ⬆️
blockchain/v2/reactor.go 36.17% <0%> (+0.85%) ⬆️
blockchain/v0/pool.go 79.16% <0%> (+0.96%) ⬆️
consensus/reactor.go 79.12% <0%> (+1.03%) ⬆️
blockchain/v2/routine.go 84.84% <0%> (+3.03%) ⬆️

@melekes melekes marked this pull request as ready for review March 5, 2020 08:14
@tac0turtle tac0turtle added C:rpc Component: JSON RPC, gRPC T:bug Type Bug (Confirmed) labels Mar 5, 2020
Copy link
Contributor

@tessr tessr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice documentation, too!

@@ -24,6 +24,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi

### BUG FIXES:

- [rpc] \#3935 Create buffered subscriptions on `/subscribe` (@melekes)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit, but do you want to make this a link like the other items in this list?

Copy link
Contributor

@tac0turtle tac0turtle Mar 5, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically this is the correct way according to the contributing.md and then there is a script to add all the links prior to release. People have been putting links recently, I guess we should decide on one way or the other?

https://github.com/tendermint/tendermint/blob/master/CONTRIBUTING.md#changelog

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, there's a script for that

@melekes melekes merged commit bc89aad into master Mar 6, 2020
@melekes melekes deleted the anton/3935-ws-subscription-issues branch March 6, 2020 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C:rpc Component: JSON RPC, gRPC T:bug Type Bug (Confirmed)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RPC subscriptions: client is not pulling messages fast enough
5 participants