Skip to content

V0.31.5#3571

Merged
ebuchman merged 10 commits intomasterfrom
v0.31
Apr 19, 2019
Merged

V0.31.5#3571
ebuchman merged 10 commits intomasterfrom
v0.31

Conversation

@ebuchman
Copy link
Contributor

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

ebuchman and others added 10 commits April 15, 2019 08:16
* [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.
Co-Authored-By: melekes <anton.kalyaev@gmail.com>
v0.31.5 changelog and version updates
@ebuchman ebuchman requested review from melekes and xla as code owners April 16, 2019 20:53
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.

👍

@codecov-io
Copy link

Codecov Report

Merging #3571 into master will decrease coverage by 0.3%.
The diff coverage is 53.57%.

@@            Coverage Diff             @@
##           master    #3571      +/-   ##
==========================================
- Coverage   64.37%   64.06%   -0.31%     
==========================================
  Files         213      213              
  Lines       17331    17381      +50     
==========================================
- Hits        11156    11136      -20     
- Misses       5256     5315      +59     
- Partials      919      930      +11
Impacted Files Coverage Δ
libs/db/mem_batch.go 92.59% <ø> (ø) ⬆️
libs/common/cmap.go 90.9% <100%> (ø) ⬆️
blockchain/reactor.go 70.56% <50%> (-8.19%) ⬇️
state/store.go 71.23% <50%> (+2.05%) ⬆️
privval/signer_validator_endpoint.go 75.55% <0%> (-10%) ⬇️
blockchain/pool.go 80.26% <0%> (-2.97%) ⬇️
consensus/reactor.go 71.31% <0%> (-1.66%) ⬇️
libs/clist/clist.go 66.66% <0%> (-1.52%) ⬇️
mempool/wire.go 100% <0%> (ø) ⬆️
... and 2 more

@ebuchman ebuchman merged commit 4253e67 into master Apr 19, 2019
@melekes melekes mentioned this pull request May 7, 2019
36 tasks
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