BFT Block Puller: BFTDeliverer #4365
Conversation
tock-ibm
force-pushed
the
bft-bp-bftdeliverer
branch
9 times, most recently
from
003e98c
to
2b6d2da
Compare
tock-ibm
force-pushed
the
bft-bp-bftdeliverer
branch
from
2b6d2da
to
3fa1fff
Compare
tock-ibm
force-pushed
the
bft-bp-bftdeliverer
branch
3 times, most recently
from
5e4a566
to
66878c4
Compare
tock-ibm
force-pushed
the
bft-bp-bftdeliverer
branch
from
66878c4
to
78d2774
Compare
| break FetchAndMonitorLoop | ||
| default: | ||
| d.Logger.Errorf("Unexpected error from CensorshipMonitor, something is critically wrong: %s", errMonitor) | ||
| break FetchAndMonitorLoop |
There was a problem hiding this comment.
Can we please not use goto style flow control?
This function is pretty big, we can extract the for loop to a separate function and just use continue.
There was a problem hiding this comment.
Not sure what you mean by goto style, these are just break of the for loop. since they are called within a select, they must be labeled. Labeling break of loops is actually recommended....
However, you are correct, this loop is indeed too big, as it does not enter a single screen... so I will break it up and try and make it more readable and simplify the flow control.
| } | ||
|
|
||
| func (d *BFTDeliverer) refreshSources() { | ||
| // select an initial source randomly | ||
| d.fetchSources = shuffle(d.Orderers.Endpoints()) | ||
| d.fetchSources = d.Orderers.ShuffledEndpoints() |
There was a problem hiding this comment.
why do we shuffle each time we refresh an endpoint?
There was a problem hiding this comment.
When even a single endpoint is changed, all the endpoints are refreshed, i.e. respective channel is closed. In that case we have a new array of sources, possibly of different length. This only happens following a config TX that actually changes the endpoints, and the first time the bft deliverer is started. That is - it happens very rarely. Following a config TX the raw source slice will be the same in all peers. Therefore, in order to avoid all peers selecting the same orderer as their block source, we have to shuffle the raw sources slice and select from the shuffled slice.
| } | ||
|
|
||
| d.censorshipMonitor.Stop() |
There was a problem hiding this comment.
why do we start everything just to tear it down? Can't we just stop one censorship monitor?
There was a problem hiding this comment.
the d.censorshipMonitor.Stop() at the end of the loop is only called when something "bad" happens, which is recoverable with a retry, that is:
- block fetch error
- censorship event
These are hopefully rare events. Therefore, for simplicity, it is easier to close all the header receivers and block receiver, and reassign everything from scratch, rather than to complicate the implementation and
- close only the block receiver and replace it with a header receive
- close a header receiver and replace it with a block receiver
Note that the censorship monitor is a collection of header receivers, yet failures in header receivers do not cause a restart of the monitor. Header receiver failures are handled within. This is because eventually, an incorrect orderer (malicious) will be assigned to a header receiver, which should be able to tolerate its behavior and not disrupt the reception of blocks.
BFT header receivers that pull headers from the orderers, and keep the time and number of the last one. The header receivers verify each block as it arrives. The header receivers will receive (or ask for) full config blocks - in a later commit. The header receivers will maintain their own private block verifier (bundle) - in a later commit. A BFT censorship monitor which periodically compares the progress of headers relative to blocks. The monitor triggers an alert if a header is ahead of the block stream for more than a certain time. The BFTDeliverer is only a skeleton Signed-off-by: Yoav Tock <tock@il.ibm.com> Change-Id: I5180a98640073b87effb478fd86b6fa7d4df5dee
tock-ibm
force-pushed
the
bft-bp-bftdeliverer
branch
2 times, most recently
from
47dcfaa
to
c945a63
Compare
- A BFTDeliverer that fetches blocks and maintains a BFTCensorshipMonitor. - Abstract the creation of a BFTCensorshipMonitor via an abstract factory, so that we can use a mock for it in testing. - Add a "shuffledEndpoints" method to the connection source and test it. - Unit testing of BFTDeliverer. Signed-off-by: Yoav Tock <tock@il.ibm.com> Change-Id: Ifead3f9e6c803c4d9fabc63acce11c6da472b88d
tock-ibm
force-pushed
the
bft-bp-bftdeliverer
branch
from
c945a63
to
308995d
Compare
Type of change
Description
Related issues
#4348