catchup: cleanup pipelinedFetch #5662
Conversation
New ledger code checks partkeys, and buildTestLedger() was creating Online accounts without any valid partkeys.
Waits for round r to appear in memory, without waiting for it to be committed to disk. Implemented by instantiating another copy of the notifier tracker, which notifies when newBlock() is called.
More streamlined code for catchup, which avoids the carefully orchestrated chain of channels, each of which was getting read twice, between each round's fetchAndWrite invocation, the nested closure created by pipelineCallback(), and the one-off implementation of task queues in taskCh. This should make the catchup code easier to modify when adding state-proof-based block validation.
Codecov Report
@@ Coverage Diff @@
## master #5662 +/- ##
==========================================
- Coverage 55.13% 55.11% -0.03%
==========================================
Files 465 465
Lines 65022 64993 -29
==========================================
- Hits 35852 35823 -29
+ Misses 26780 26779 -1
- Partials 2390 2391 +1
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Pass context.Context as the first argument to fetchAndWrite
zeldovich
dismissed stale reviews from algonautshant and algorandskiy
via
d4f29ee
|
catchup test TestOnSwitchToUnSupportedProtocol2 failed after merging this |
|
@zeldovich it is reproducible - hangs after adding block 10 |
|
Huh, will take a look now; thanks for pointing this out. |
|
Most likely this test is no longer needed. It is for "the rest" loop, but it has been eliminated. |
|
There's a bit of a race condition in the new loop. The test might not be quite so important, and I think the race is fairly benign, but still it seems like a good idea to make the test pass. The tests did pass for me on my machine at some point, but I didn't realize what test partitioning was doing, and I thought CI kept running all of the tests on each run, but apparently not. Oops. In any case, I'll make this more deterministic and submit another PR shortly. |
|
(The fix is #5673) |
This PR simplifies the catchup code for managing a pipeline of block-fetching workers. Catchup should work in more-or-less the same way as before, but the code itself should be easier to read and modify.