Skip to content

v0.31.5 changelog and version updates#3568

Merged
ebuchman merged 11 commits intov0.31from
anton/release-v0.31.5
Apr 16, 2019
Merged

v0.31.5 changelog and version updates#3568
ebuchman merged 11 commits intov0.31from
anton/release-v0.31.5

Conversation

@melekes
Copy link
Contributor

@melekes melekes commented Apr 16, 2019

ebuchman and others added 10 commits April 2, 2019 19:18
* [adr] Peer behaviour adr updates
* [docs] fix Behaved function signature
* [adr] typo fix in code example
What happened:

New code was supposed to fall back to last height changed when/if it
failed to find validators at checkpoint height (to make release
non-breaking).

But because we did not check if validator set is empty, the fall back
logic was never executed => resulting in LoadValidators returning an
empty validator set for cases where `lastStoredHeight` is checkpoint
height (i.e. almost all heights if the application does not change
validator set often).

How it was found:

one of our users - @sunboshan reported a bug here
#3537 (comment)

* use last height changed in validator set is empty
* add a changelog entry
Fixes #3457

The topic of the issue is that : write a BlockRequest int requestsCh channel will create an timer at the same time that stop the peer 15s later if no block have been received . But pop a BlockRequest from requestsCh and send it out may delay more than 15s later. So that the peer will be stopped for error("send nothing to us").
Extracting requestsCh into its own goroutine can make sure that every BlockRequest been handled timely.

Instead of the requestsCh handling, we should probably pull the didProcessCh handling in a separate go routine since this is the one "starving" the other channel handlers. I believe the way it is right now, we still have issues with high delays in errorsCh handling that might cause sending requests to invalid/ disconnected peers.
@melekes melekes requested review from ebuchman and xla as code owners April 16, 2019 09:39
@codecov-io
Copy link

codecov-io commented Apr 16, 2019

Codecov Report

Merging #3568 into v0.31 will decrease coverage by 0.24%.
The diff coverage is 60.71%.

@@            Coverage Diff             @@
##            v0.31    #3568      +/-   ##
==========================================
- Coverage   64.28%   64.04%   -0.25%     
==========================================
  Files         213      213              
  Lines       17374    17381       +7     
==========================================
- Hits        11169    11131      -38     
- Misses       5286     5317      +31     
- Partials      919      933      +14
Impacted Files Coverage Δ
libs/db/mem_batch.go 92.59% <ø> (ø) ⬆️
libs/common/cmap.go 90.9% <100%> (ø) ⬆️
state/store.go 71.23% <50%> (+2.05%) ⬆️
blockchain/reactor.go 71.49% <58.33%> (-0.97%) ⬇️
privval/signer_validator_endpoint.go 75.55% <0%> (-10%) ⬇️
privval/signer_service_endpoint.go 85.45% <0%> (-3.64%) ⬇️
privval/socket_listeners.go 86.2% <0%> (-3.45%) ⬇️
consensus/reactor.go 70.36% <0%> (-2.37%) ⬇️
blockchain/pool.go 81.25% <0%> (-1.32%) ⬇️
p2p/pex/pex_reactor.go 78.83% <0%> (-1.23%) ⬇️
... and 2 more

*April 16th, 2019*

This release fixes a regression from v0.31.4 where, in existing chains that
were upgraded, `/validators` could return an empty validator set. This is true
Copy link
Contributor

@liamsi liamsi Apr 16, 2019

Choose a reason for hiding this comment

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

Suggestion:
"This release fixes a regression from v0.31.4 where, in existing chains that
were upgraded, querying /validators via RPC could return an empty validator set.
This happened for almost all heights, given the validator set remained the same."
?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like my version better :) I'll let @ebuchman be the judge

Copy link
Contributor

Choose a reason for hiding this comment

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

Ismail's is defntly clearer that this is about the RPC endpoint, though we often refer to /xxx and expect the reader to know we're talking about the RPC.

Copy link
Contributor

@liamsi liamsi left a comment

Choose a reason for hiding this comment

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

Thanks @melekes 🙌 I left a few nits and suggestions regarding the changelog.

Note: the last two releases were prepared and directly based on (and later merged to) the release branch v0.31. The changes in the changelog got to develop by merging back from v0.31.
I would suggest to stick with this procedure for now and base this on the release branch. We can discuss pros and cons and what works best in the future offline (especially, when we finally get rid of one of the branches, master, develop).

@liamsi liamsi changed the base branch from develop to v0.31 April 16, 2019 11:05
Co-Authored-By: melekes <anton.kalyaev@gmail.com>
@ebuchman ebuchman merged commit d2eab53 into v0.31 Apr 16, 2019
@ebuchman ebuchman deleted the anton/release-v0.31.5 branch April 16, 2019 20:52
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.

7 participants