Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

dispute-coordinator: always import trusted votes #4793

Closed
sandreim opened this issue Jan 27, 2022 · 5 comments
Closed

dispute-coordinator: always import trusted votes #4793

sandreim opened this issue Jan 27, 2022 · 5 comments
Assignees
Labels
I8-refactor Code needs refactoring. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.

Comments

@sandreim
Copy link
Contributor

Currently we need the SessionInfo to check signatures in handle_import_statements when importing votes. This is because the generality of the approach when trying to treat all votes as the same. However, for all backing, approval, own votes and all votes scraped from chain we don't need the session info, so we can split handle_import_statements such that it calls an inner function that imports them.

@eskimor
Copy link
Member

eskimor commented Feb 1, 2022

For the import of explicit votes (coming from the network), we should provide an API (message type), that makes use of DisputeMessage.

This way we maintain the type level guarantee that a dispute can only be raised if there is at least one valid and one invalid vote & we can drop signature checking even in that case as DisputeMessage is already checked.

@sandreim
Copy link
Contributor Author

@eskimor are you already working on this as part of the dispute-coordinator refactoring ?

@eskimor
Copy link
Member

eskimor commented Apr 25, 2022

Hi @sandreim ! Actually there are no signatures being checked. An overhaul of the API might still make sense, to simplify the implementation. I am not currently working on this, so feel free.

@eskimor
Copy link
Member

eskimor commented Apr 25, 2022

One thing that 5368 makes clear immediately: We do have lots of import votes calls, which makes sense, as we will have roughly 1/2 per parachain per block in backing and also a lot during approval voting.

So any refactor should take that into account and try to batch those imports together in some smart way/or make the call super cheap by some other means.

The calls themselves are already relatively quick: https://grafana.parity-mgmt.parity.io/goto/4MZY_xwnk?orgId=1

All other requests have way less frequency. Appart from active leaves update, import is the heaviest call and the most frequent - by orders of magnitude.

@eskimor
Copy link
Member

eskimor commented Apr 25, 2022

The most frequent db operation is load recent disputes: https://grafana.parity-mgmt.parity.io/goto/6vrVubw7z?orgId=1 and candidate votes (not surprisingly), as those are loaded in statement import. So we have around 80 such loads per second on Versi right now. Writes are in the same ballpark. The heaviest, is loading candidate votes (taking into account duration * frequency) - We are in the range of <10ms per second for all requests combined.

db-ops-sum

We can see here that import statements, takes around 6ms/s (duration * number of requests).

requests-sum

This suggests, that the actual cost of the call are indeed db operations. (As they amount in total to more time than the import requests themselves.) So I would be very surprised, if a PR like this one would not have a big effect on performance.

@eskimor eskimor closed this as completed Aug 16, 2022
@ordian ordian added the T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance. label Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I8-refactor Code needs refactoring. T4-parachains_engineering This PR/Issue is related to Parachains performance, stability, maintenance.
Projects
None yet
Development

No branches or pull requests

3 participants