llmq: Some fixes/improvements #3943

xdustinface · 2021-01-17T14:58:52Z

No description provided.

src/llmq/quorums.cpp

src/llmq/quorums_blockprocessor.cpp

src/llmq/quorums_utils.h

There should be no reason to keep this tread attached to its parent, if so, let me know.

xdustinface · 2021-01-20T05:07:14Z

@UdjinM6 See the last 5 commits, i changed/dropped some stuff. Quorums/Commitment caches are now unordered_lru_cache and scanQuorumsCache cache size is fixed by just calculating it based on the params.

src/llmq/quorums_utils.h

src/llmq/quorums.h

xdustinface · 2021-01-21T02:02:32Z

Okay so, i pushed some more commits to refactor ScanQuorums a bit because IMO it was really hard to read/understand its full logic before (and i hope i didn't break any of its logic haha). Also it seems like it didn't reuse existing cache if more than cache size was requested. So let's say we had 10 quorums in cache and you requested 11 it read all 11 from DB instead of 1 (0603e98)

This recent commits also changes the way the scanQuorumCache works from just one unordered_lru_cache to one unordered_lru_cache for each LLMQ type like with the normal quorums and commitments caches (
4f2a804). The rest should be obvious.

UdjinM6 · 2021-01-21T18:05:35Z

LGTM so far, waiting for the "inactive quorum" issue to be discussed (and fixed if needed).

xdustinface · 2021-01-22T05:15:25Z

Okay so in the meeting today it turned out that the reason for querying old quorums is mainly to validate IS locks with verifyislock and chainlocks with quorum verify. But it appears that this only happens frequently during sync of platform because they store the the IS/CL in the platform blocks and they need to validate them on sync. When it's synced it can still happen but unlikely/not often and thats only if someone submits an asset lock older than our singing window to platform. I can't tell you much more details here so if you have more questions i guess @antouhou or @shumkov could leave you an answer.

Soo, from my understanding it wouldn't help a lot then here to have a bigger cache, right? We could maybe still just add a command line which allows to increase the cache sizes...not sure if it makes sense, but we could haha..

And i guess someone (i would but i don't know when :P) should in any case do some research and testing around all this to see how resource consuming fun like this is also given the fact that ivan said they are sometimes at the limit of the RPC queue during sync 🤔

UdjinM6

utACK

PastaPastaPasta

utACK, one nit

PastaPastaPasta · 2021-01-25T03:14:48Z

src/llmq/quorums_utils.h

+    template <typename CacheType>
+    static void InitQuorumsCache(CacheType& cache)
+    {
+        for (auto& llmq : Params().GetConsensus().llmqs) {


Can llmq be const?

You could add it, but llmq is already const because GetConsensus() returns a const reference so llmqs is const and it would be kind of redundant.

* llmq: Detach dash-q-cachepop from caller There should be no reason to keep this tread attached to its parent, if so, let me know. * llmq: Avoid nullptr access for pindexStart in ScanQuorums * llmq: Add cacheKey in ProcessCommitment * llmq: Erase minable commitments if they have been processed * llmq: Add CLLMQUtils::InitQuorumsCache * llmq: Use unordered_lru_cache for quorumsCache and rename it * llmq: Use unordered_lru_cache for hasMinedCommitmentCache and rename it * llmq: Drop redundant check * llmq: Rename nMaxCount2 -> nScanCommitments * llmq: Refactor storeCache -> fCacheExists * llmq: Rename maxCount -> nCountRequested * llmq: Rename result -> vecResultQuorums * llmq: Return an empty vector if the are zero elements requested * unordered_lru_cache: Add max_size() * llmq: Partially reuse existing cache if more than max is requested * llmq: std::map<LLMQType, unordered_lru_cache<...>> for scanQuoumsCache * llmq: Drop params * llmq: Only emplace to cache if there is something available

xdustinface added this to the 17 milestone Jan 17, 2021

xdustinface force-pushed the pr-llmq-some-fixes branch from fc28871 to 2fc079c Compare January 17, 2021 20:05

xdustinface marked this pull request as ready for review January 17, 2021 21:35

UdjinM6 requested changes Jan 18, 2021

View reviewed changes

src/llmq/quorums.cpp Outdated Show resolved Hide resolved

src/llmq/quorums_blockprocessor.cpp Outdated Show resolved Hide resolved

src/llmq/quorums_utils.h Outdated Show resolved Hide resolved

xdustinface marked this pull request as draft January 18, 2021 23:07

xdustinface mentioned this pull request Jan 19, 2021

llmq|rpc|test|version: Implement P2P messages QGETDATA <-> QDATA #3953

Merged

xdustinface added 4 commits January 19, 2021 22:19

llmq: Detach dash-q-cachepop from caller

701140d

There should be no reason to keep this tread attached to its parent, if so, let me know.

llmq: Avoid nullptr access for pindexStart in ScanQuorums

b8e0cd2

llmq: Add cacheKey in ProcessCommitment

9553435

llmq: Erase minable commitments if they have been processed

df9607c

xdustinface force-pushed the pr-llmq-some-fixes branch 2 times, most recently from 186c12c to a47ccac Compare January 20, 2021 04:04

xdustinface marked this pull request as ready for review January 20, 2021 05:07

xdustinface requested a review from PastaPastaPasta January 20, 2021 05:08

PastaPastaPasta requested a review from UdjinM6 January 20, 2021 16:34

UdjinM6 reviewed Jan 20, 2021

View reviewed changes

src/llmq/quorums_utils.h Outdated Show resolved Hide resolved

src/llmq/quorums_utils.h Outdated Show resolved Hide resolved

src/llmq/quorums.h Outdated Show resolved Hide resolved

xdustinface added 13 commits January 21, 2021 01:49

llmq: Add CLLMQUtils::InitQuorumsCache

5856cce

llmq: Use unordered_lru_cache for quorumsCache and rename it

0bb983a

llmq: Use unordered_lru_cache for hasMinedCommitmentCache and rename it

3da8cbf

llmq: Drop redundant check

c595cd8

llmq: Rename nMaxCount2 -> nScanCommitments

3c473b6

llmq: Refactor storeCache -> fCacheExists

22d33b5

llmq: Rename maxCount -> nCountRequested

561ca08

llmq: Rename result -> vecResultQuorums

3ca5493

llmq: Return an empty vector if the are zero elements requested

2ca980f

unordered_lru_cache: Add max_size()

ffcddd5

llmq: Partially reuse existing cache if more than max is requested

0603e98

llmq: std::map<LLMQType, unordered_lru_cache<...>> for scanQuoumsCache

4f2a804

llmq: Drop params

fc7db3a

llmq: Only emplace to cache if there is something available

dbc08f0

xdustinface force-pushed the pr-llmq-some-fixes branch from a47ccac to dbc08f0 Compare January 21, 2021 01:38

UdjinM6 approved these changes Jan 22, 2021

View reviewed changes

PastaPastaPasta approved these changes Jan 25, 2021

View reviewed changes

xdustinface merged commit dc2473c into dashpay:develop Jan 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

llmq: Some fixes/improvements #3943

llmq: Some fixes/improvements #3943

Uh oh!

xdustinface commented Jan 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xdustinface commented Jan 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xdustinface commented Jan 21, 2021

Uh oh!

UdjinM6 commented Jan 21, 2021

Uh oh!

xdustinface commented Jan 22, 2021

Uh oh!

UdjinM6 left a comment

Uh oh!

PastaPastaPasta left a comment

Uh oh!

PastaPastaPasta Jan 25, 2021

Uh oh!

xdustinface Jan 25, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

llmq: Some fixes/improvements #3943

llmq: Some fixes/improvements #3943

Uh oh!

Conversation

xdustinface commented Jan 17, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xdustinface commented Jan 20, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

xdustinface commented Jan 21, 2021

Uh oh!

UdjinM6 commented Jan 21, 2021

Uh oh!

xdustinface commented Jan 22, 2021

Uh oh!

UdjinM6 left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta left a comment

Choose a reason for hiding this comment

Uh oh!

PastaPastaPasta Jan 25, 2021

Choose a reason for hiding this comment

Uh oh!

xdustinface Jan 25, 2021

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants