Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Faster sync #6563

Merged
merged 9 commits into from
Nov 5, 2020
Merged

Faster sync #6563

merged 9 commits into from
Nov 5, 2020

Conversation

karlb
Copy link
Contributor

@karlb karlb commented Sep 25, 2020

Improve event syncing speed by filtering more selectively. This brings down the initial sync on mainnet for a fresh node to 30s (parity, infura) to 2mins (geth).

Before

Fetch all events for the relevant contracts and filter in the Raiden client

After

  • Fetch TokenNetworkCreated events from TN registry
  • For each TN found, fetch the ChannelOpened events that include the node as participant
  • For each of these channels, fetch the ChannelClosed, ChannelSettled, ChannelNewDeposit and ChannelWithdraw events
  • Fetch all SecretRevealed events from the SecretRegistry once the node starts to participate in a channel

Closes #6497

The SP integration test must fail until raiden-network/scenario-player#635 is merged, which has to be based on this PR. To break the cycle, I will force merge this when all tests but the SP integration are green.

Potential future optimizations if we need more speed:

  • We could only query for secrets with a specific secrethash
  • We could stop looking for new token networks one the registries limit is reached (we don't want to have limits in the long run, though)
  • We could get some information form a thegraph.com subgraph
  • We could look for new TNs in the background while we allow working on the found TNs
  • We could save a list of tokens in the TN registry contract, so that we don't have to fetch the events

karlb added a commit to karlb/scenario-player that referenced this pull request Oct 30, 2020
The selective event filtering introduced in
raiden-network/raiden#6563 changed the API,
which make this commit necessary.
@karlb karlb marked this pull request as ready for review October 30, 2020 09:44
Copy link
Contributor

@fredo fredo left a comment

Choose a reason for hiding this comment

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

LGTM, just a typo and a question left.


def union(self, other: "RaidenContractFilter") -> "RaidenContractFilter":
""" Return a new RaidenContractFilter with all elements from both input filters """
# We must not have to different non-None secret registries. Choose the non-None one.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# We must not have to different non-None secret registries. Choose the non-None one.
# We must not have two different non-None secret registries. Choose the non-None one.

@@ -318,21 +318,6 @@ def token_network_address(self) -> TokenNetworkAddress:
# FIXME: remove on next breaking release
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need this comment?

Copy link
Contributor

Choose a reason for hiding this comment

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

This (ContractReceiveRouteClosed) shouldn't be removed if the released is supposed to be backwards-compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

right, but it is removed in this PR isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's my point. Depending on what the goals are, this should eventually stay.

palango and others added 9 commits November 5, 2020 11:20
That script belongs to
#6209, but we chose to do
#6497, instead.
Instead of requesting all events for the relevant contracts, we filter
much more selectively by including the right topics.
Apparently, it is faster to include a topic, event when you want to
filter for all events of a contract.
If we don't have a channel, we are not interested in any secrets
registered online. Thus we can skip the slow event log queries until we
open our first channel.
For each block range, we make multiple requests with different filters.
Before this commit, only the last request duration has been used to
adjust the batch size. But the point of the adjustment is to prevent any
request from running into a timeout, so adjusting based on the maximal
duration of all requests is a better way to achieve this.
After working with this a bit, the defaultdict seemed to make things
less clear than a using a normal dict with `get` and `setdefault`.
@karlb karlb merged commit 97a68f5 into raiden-network:develop Nov 5, 2020
karlb added a commit to karlb/scenario-player that referenced this pull request Nov 5, 2020
The selective event filtering introduced in
raiden-network/raiden#6563 changed the API,
which make this commit necessary.
karlb added a commit to karlb/scenario-player that referenced this pull request Nov 5, 2020
The selective event filtering introduced in
raiden-network/raiden#6563 changed the API,
which make this commit necessary.
@karlb karlb deleted the faster-sync branch November 5, 2020 12:47
palango pushed a commit to raiden-network/scenario-player that referenced this pull request Nov 5, 2020
The selective event filtering introduced in
raiden-network/raiden#6563 changed the API,
which make this commit necessary.
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.

Faster syncing during startup by more selective filtering
3 participants