Beam Sync v0.1 #641

carver · 2019-05-20T22:41:17Z

What was wrong?

It takes too long to sync. Beam sync is an experiment to see if we can direct fast sync to choose the nodes we need in order to execute the latest blocks in the chain. If we can download the needed state fast enough, then we can keep up with the tip of the chain as soon as we find the right header tip. (Right now, we still have to commit all headers before starting beam sync, which would become the new longest part of sync).

How was it fixed?

Depends on ethereum/py-evm#1783

A lot of hacked stuff. This is never intended to be merged as is, except for a few pieces that I'll split off to independent PRs. It's got some horrendous code, that was just built for prototyping. The reason for publishing the PR is to provide some context to where Beam Sync is headed.

There is a beam sync test added, with a custom database. That should likely make it into a final Beam Sync PR.

Next steps are to re-implement in a different approach: Subclass VM, catch exceptions while getting state, pause execution until the state is added to the DB, then resume. That should provide some speed benefits, plus require less custom stuff in py-evm (like MidBlockState).

Ripping pieces out, into:

TODO

Probably later, at least write up an issue:

bugfix: properly handle uncle validation in beam sync (download/import previous six blocks?) Beam Sync: Uncle Validation #717
upgrade to lahja 0.14 API?
add statistics of beam execution data downloads and timing, by block Beam Sync: stats #718
~~move beam sync run_in_executor to its own process instead of thread? (not critical, because only one block imported at a time right now)~~

Cute Animal Picture

carver · 2019-05-20T22:43:34Z

cc @pipermerriam for some context on beam sync

pipermerriam · 2019-05-21T02:05:56Z

tests/core/integration_test_helpers.py

@@ -148,7 +155,7 @@ def load_mining_chain(db):

    return build(
        FakeAsyncChain,
-        byzantium_at(0),
+        petersburg_at(0),


side note

Might be nice to have a latest_mainnet_fork_at that keeps us from having to propagate these updates through the codebase.

pipermerriam

😎

trinity/sync/beam/chain.py

pipermerriam · 2019-05-21T12:31:53Z

   ERROR  05-20 22:34:04  StateRetrievingBlockImporter  Unexpected exception during speculative execution
Traceback (most recent call last):
  File "/home/piper/projects/trinity/trinity/sync/beam/chain.py", line 383, in _execute_speculate_transaction
    vm.apply_all_transactions((transaction, ), base_header_for_import)
  File "/home/piper/projects/py-evm/eth/vm/base.py", line 553, in apply_all_transactions
    result_header = self.add_receipt_to_header(previous_header, receipt)
UnboundLocalError: local variable 'receipt' referenced before assignment

May be irrelevant depending on how you're refactoring but I ran into this last night.

carver · 2019-05-21T17:07:54Z

   ERROR  05-20 22:34:04  StateRetrievingBlockImporter  Unexpected exception during speculative execution
Traceback (most recent call last):
  File "/home/piper/projects/trinity/trinity/sync/beam/chain.py", line 383, in _execute_speculate_transaction
    vm.apply_all_transactions((transaction, ), base_header_for_import)
  File "/home/piper/projects/py-evm/eth/vm/base.py", line 553, in apply_all_transactions
    result_header = self.add_receipt_to_header(previous_header, receipt)
UnboundLocalError: local variable 'receipt' referenced before assignment

May be irrelevant depending on how you're refactoring but I ran into this last night.

Maybe you weren't using the branch of py-evm from ethereum/py-evm#1783 ?

pipermerriam · 2019-05-21T18:03:40Z

Yup, wrong py-evm branch. I clearely didn't look at that stack trace very closely because when I posted I thought it was trinity code...

trinity/sync/beam/chain.py

+            self._transaction_completed[block.hash] = {}
+            for txn_idx in range(len(block.transactions)):
+                self._transaction_completed[block.hash][txn_idx] = asyncio.Event()
+                self._state_downloader.run_task(self._speculate_transaction(txn_idx, block))


tests/core/integration_test_helpers.py

trinity/protocol/eth/servers.py

carver · 2019-06-08T00:29:06Z

Whelp, cut the PR down from +2.?k lines to +1.4k. -4 lines, lol

Currently failing the beam sync tests because I haven't updated to the new request server and peer pool approach in test_sync After that, and dealing with uncle validation, and maybe some other unknown lahja stuff (trying to put that off until 0.14), then I think it should be working again. After adding some docstrings and running against the full suite of beam tests (not just the 4 selected ones that I intend to commit) then this should ready for a final review.

I know it's huge. Sorry. Open to more ideas on how to break this up.

carver · 2019-06-12T00:06:25Z

@pipermerriam okay, everything's passing. No major updates on functionality since the last run-through, just been working on getting it merge-ready. Can you take a look?

The biggest flaw right now is the uncle validation handling. It dies if you try syncing to a block with an uncle right now. It's fixable, but I think the fix belongs in its own PR, since this one is already huge.

carver · 2019-06-12T23:32:15Z

Okay, this one is really, actually passing this time. (Well it was before, but stochastically)

pipermerriam

Frankly, this is large enough that I'm good moving it forward however you see fit and we can address any architectural issues iteratively. It's kind of in that "too big" range where I'm mostly inclined to trust you under the premise that I'll grow to understand it over time and we'll probably end up re-writing every piece of this iteratively both as it evolves and as we migrate to trio.

trinity/sync/beam/chain.py

trinity/sync/beam/importer.py

trinity/sync/beam/state.py

carver · 2019-06-13T21:47:42Z

Frankly, this is large enough that I'm good moving it forward however you see fit and we can address any architectural issues iteratively. It's kind of in that "too big" range where I'm mostly inclined to trust you under the premise that I'll grow to understand it over time and we'll probably end up re-writing every piece of this iteratively both as it evolves and as we migrate to trio.

Ok, cool. Totally understood. I'll see if I can pair review with @lithp for a bit for a second look. The good news is that everything is behind an opt-in sync-mode flag, so it's pretty low risk, even going into a release as-is.

carver · 2019-06-14T00:40:41Z

Also from pair review with Brian:

Explain more about why await import_completion prevents test hanging
File the TODO issues about the get node data requests: GetNodeData exchange: validate incoming node hashes #719 and GetNodeData EthRequestServer should not return duplicate nodes #720
- accepting duplicate nodes
- accepting empty node hashes
- server replying with duplicate nodes
Rename StateDataHunter to something like: BeamDownloadDriver, ...DownloadHandler, ...DataEventHandler, etc.

Beam sync will fail if any of the first 6 blocks have uncles. A solution for that is in progress and will be posted shortly.

pipermerriam · 2019-06-14T14:10:05Z

HUZZAH!

carver force-pushed the beam-sync branch from 2f23695 to de7c287 Compare May 20, 2019 22:45

pipermerriam reviewed May 21, 2019

View reviewed changes

trinity/sync/beam/chain.py Outdated Show resolved Hide resolved

carver commented May 25, 2019

View reviewed changes

carver force-pushed the beam-sync branch from de7c287 to 9d5d704 Compare May 30, 2019 00:40

This was referenced Jun 6, 2019

Update sync tests to latest VM, where possible #692

Merged

Add a maximum batch size for regular sync imports #704

Merged

carver force-pushed the beam-sync branch from 9d5d704 to 750c5df Compare June 7, 2019 22:25

carver changed the title ~~[Hacked Prototype] Beam Sync~~ [WIP] Beam Sync Jun 7, 2019

carver force-pushed the beam-sync branch from 750c5df to add1c61 Compare June 7, 2019 23:53

carver commented Jun 8, 2019

View reviewed changes

tests/core/integration_test_helpers.py Outdated Show resolved Hide resolved

carver commented Jun 8, 2019

View reviewed changes

trinity/protocol/eth/servers.py Show resolved Hide resolved

carver marked this pull request as ready for review June 11, 2019 23:45

carver changed the title ~~[WIP] Beam Sync~~ Beam Sync v0.1 Jun 12, 2019

carver force-pushed the beam-sync branch from c2e47cf to 773ce2c Compare June 12, 2019 22:58

carver requested a review from pipermerriam June 12, 2019 23:31

pipermerriam approved these changes Jun 13, 2019

View reviewed changes

carver mentioned this pull request Jun 14, 2019

Beam Sync: Uncle Validation #717

Closed

carver force-pushed the beam-sync branch from 773ce2c to fd39841 Compare June 14, 2019 03:44

First Beam Sync prototype

f40f4c8

Beam sync will fail if any of the first 6 blocks have uncles. A solution for that is in progress and will be posted shortly.

carver force-pushed the beam-sync branch from fd39841 to f40f4c8 Compare June 14, 2019 04:33

carver added a commit that referenced this pull request Jun 14, 2019

Add release notes #641

93a151b

carver merged commit af4bc6b into ethereum:master Jun 14, 2019

carver deleted the beam-sync branch June 14, 2019 14:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Beam Sync v0.1 #641

Beam Sync v0.1 #641

carver commented May 20, 2019 •

edited

Loading

carver commented May 20, 2019

pipermerriam May 21, 2019

pipermerriam left a comment

pipermerriam commented May 21, 2019

carver commented May 21, 2019

pipermerriam commented May 21, 2019

This comment was marked as outdated.

carver commented Jun 8, 2019 •

edited

Loading

carver commented Jun 12, 2019

carver commented Jun 12, 2019

pipermerriam left a comment

carver commented Jun 13, 2019

carver commented Jun 14, 2019 •

edited

Loading

pipermerriam commented Jun 14, 2019

Beam Sync v0.1 #641

Beam Sync v0.1 #641

Conversation

carver commented May 20, 2019 • edited Loading

What was wrong?

How was it fixed?

TODO

Probably later, at least write up an issue:

Cute Animal Picture

carver commented May 20, 2019

pipermerriam May 21, 2019

Choose a reason for hiding this comment

pipermerriam left a comment

Choose a reason for hiding this comment

pipermerriam commented May 21, 2019

carver commented May 21, 2019

pipermerriam commented May 21, 2019

This comment was marked as outdated.

carver commented Jun 8, 2019 • edited Loading

carver commented Jun 12, 2019

carver commented Jun 12, 2019

pipermerriam left a comment

Choose a reason for hiding this comment

carver commented Jun 13, 2019

carver commented Jun 14, 2019 • edited Loading

pipermerriam commented Jun 14, 2019

carver commented May 20, 2019 •

edited

Loading

carver commented Jun 8, 2019 •

edited

Loading

carver commented Jun 14, 2019 •

edited

Loading