[Merged by Bors] - Optimize searching for positioning ATX #5952
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5952 +/- ##
=======================================
Coverage 80.8% 80.8%
=======================================
Files 287 287
Lines 29688 29742 +54
=======================================
+ Hits 24010 24058 +48
- Misses 4109 4118 +9
+ Partials 1569 1566 -3 ☔ View full report in Codecov by Sentry. |
| case id != b.conf.GoldenATXID: | ||
| if candidate, err := atxs.Get(b.db, id); err == nil { | ||
| if previous.TickHeight() >= candidate.TickHeight() { | ||
| id = previous.ID() | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I am not sure if this check is necessary? If we have a previous ATX and the search found something then this will always be the previous ATX or one with a higher TickHeight, won't it be? I get the intention of preferring one's own ATX in the case that the candidate is a different ATX with the exact same TickHeight but I don't think that this is strictly advantageous? The candidate has already been verified to be valid.
There was a problem hiding this comment.
The search in atxsdata doesn't prefer own ATXs - it picks the first of the highest randomly. The check here ensures we pick our own ATX.
I get the intention of preferring one's own ATX in the case that the candidate is a different ATX with the exact same TickHeight but I don't think that this is strictly advantageous?
The only advantage I see is that it's "less risky" to refer to something created by self. The existing code does the same and I wanted to keep the functionality unchanged.
There was a problem hiding this comment.
The check here ensures we pick our own ATX.
only in the case of a tie. Even if the node decided candidate is valid although it isn't this has no impact on ATXs that use it as a positioning ATX. As long as it is syntactically valid it can be referenced even if malfeasant. It has to be like this or marking an identity as malfeasant would cause everyone who referenced them (directly or indirectly) to fail validation of their published ATXs.
I'm not blocking the merge of this PR because of this, just making sure we are on the same page 🙂
Co-authored-by: Matthias Fasching <[email protected]>
|
bors merge |
## Motivation Searching for positioning ATX is slow because: - the SQL query is slow - it usually happens at the same time as many ATXs are being inserted into the DB (the poet CG)
|
Build failed (retrying...): |
## Motivation Searching for positioning ATX is slow because: - the SQL query is slow - it usually happens at the same time as many ATXs are being inserted into the DB (the poet CG)
|
Pull request successfully merged into develop. Build succeeded: |
spacemesh-bors
bot
changed the title
Searching for positioning ATX is slow because: - the SQL query is slow - it usually happens at the same time as many ATXs are being inserted into the DB (the poet CG)
Searching for positioning ATX is slow because: - the SQL query is slow - it usually happens at the same time as many ATXs are being inserted into the DB (the poet CG)
Searching for positioning ATX is slow because: - the SQL query is slow - it usually happens at the same time as many ATXs are being inserted into the DB (the poet CG)
Motivation
Searching for positioning ATX is slow because:
Description
💡 For some users the searching took even 1h+, which resulted in being late for the poet round.
Changed the ATX Builder to use the in-memory ATX store, that contains ATXs for the last ~2 epochs to lookup a high-tick ATX for the positioning ATX.
The search algorithm is simplified and just picks the highest valid ATX. It's later decided whether to use this one or the own previous ATX (if they have equal height).
Test Plan
Added few tests, but mostly the existing tests cover the same usecase
TODO