moves shreds deduper to shred-sigverify stage #30786

behzadnouri · 2023-03-18T15:46:22Z

Problem

Shreds arriving at tvu/tvu_forward/repair sockets are each processed in
a separate thread, and since each thread has its own deduper, the
duplicates across these sockets are not filtered out.
Using a common deduper across these threads will require an RwLock
wrapper and may introduce lock contention.

Summary of Changes

The commit instead moves the shred-deduper to shred-sigverify-stage
where all these shreds arrive through the same channel.

Instead the thread-pool is passed explicitly from higher in the call stack so that: solana-labs#30786 can use the same thread-pool for shred deduplication.

Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication.

If turbine_disabled is true, the commit discards turbine packets earlier in the pipeline so that they won't interfere with the deduper and the packets can get through once turbine is enabled again. This is a pre-requisite of: solana-labs#30786 so that local-cluster tests pass.

If turbine_disabled is true, the commit discards turbine packets earlier in the pipeline so that they won't interfere with the deduper and the packets can get through once turbine is enabled again. This is a prerequisite of: solana-labs#30786 so that local-cluster tests pass.

Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication.

If turbine_disabled is true, the commit discards turbine packets earlier in the pipeline so that they won't interfere with the deduper and the packets can get through once turbine is enabled again. This is a prerequisite of: solana-labs#30786 so that local-cluster tests pass.

Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication.

Instead the thread-pool is passed explicitly from higher in the call stack so that #30786 can use the same thread-pool for shred deduplication.

If turbine_disabled is true, the commit discards turbine packets earlier in the pipeline so that they won't interfere with the deduper and the packets can get through once turbine is enabled again. This is a prerequisite of: #30786 so that local-cluster tests pass.

Shreds arriving at tvu/tvu_forward/repair sockets are each processed in a separate thread, and since each thread has its own deduper, the duplicates across these sockets are not filtered out. Using a common deduper across these threads will require an RwLock wrapper and may introduce lock contention. The commit instead moves the shred-deduper to shred-sigverify-stage where all these shreds arrive through the same channel.

Instead the thread-pool is passed explicitly from higher in the call stack so that #30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf) # Conflicts: # core/src/sigverify_shreds.rs # ledger/src/sigverify_shreds.rs

If turbine_disabled is true, the commit discards turbine packets earlier in the pipeline so that they won't interfere with the deduper and the packets can get through once turbine is enabled again. This is a prerequisite of: #30786 so that local-cluster tests pass. (cherry picked from commit e66edeb) # Conflicts: # core/src/shred_fetch_stage.rs

codecov · 2023-03-21T16:50:36Z

Codecov Report

Merging #30786 (bc280c3) into master (bc933c6) will increase coverage by 0.0%.
The diff coverage is 47.9%.

@@           Coverage Diff           @@
##           master   #30786   +/-   ##
=======================================
  Coverage    81.4%    81.4%           
=======================================
  Files         725      725           
  Lines      203596   203602    +6     
=======================================
+ Hits       165833   165866   +33     
+ Misses      37763    37736   -27

…t of #30787) (#30827) * removes lazy-static thread-pool from sigverify-shreds (#30787) Instead the thread-pool is passed explicitly from higher in the call stack so that #30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf) # Conflicts: # core/src/sigverify_shreds.rs # ledger/src/sigverify_shreds.rs * resolves mergify merge conflicts --------- Co-authored-by: behzad nouri <behzadnouri@gmail.com>

…30799) (#30828) * moves turbine-disabled check to shred-fetch-stage (#30799) If turbine_disabled is true, the commit discards turbine packets earlier in the pipeline so that they won't interfere with the deduper and the packets can get through once turbine is enabled again. This is a prerequisite of: #30786 so that local-cluster tests pass. (cherry picked from commit e66edeb) # Conflicts: # core/src/shred_fetch_stage.rs * resolves mergify merge conflicts --------- Co-authored-by: behzad nouri <behzadnouri@gmail.com>

…t of solana-labs#30787) removes lazy-static thread-pool from sigverify-shreds solana-labs#30787 Instead the thread-pool is passed explicitly from higher in the call stack so that solana-labs#30786 can use the same thread-pool for shred deduplication. (cherry picked from commit c6e7aaf)

t-nelson

any concerns here wrt extra load on the shred sigverify channel? afaict we're going from up to one copy of the shred per source to all sanitized shreds going over that channel. should we consider introducing some load shedding?

behzadnouri · 2023-03-21T23:22:59Z

any concerns here wrt extra load on the shred sigverify channel? afaict we're going from up to one copy of the shred per source to all sanitized shreds going over that channel. should we consider introducing some load shedding?

hmmm, that is not change compared to before.
Before duplicates were only marked as discard but they were still sent down the channel anyways:
https://github.com/solana-labs/solana/blob/d4a6e00ff/core/src/shred_fetch_stage.rs#L103-L123
So same number of packets are sent through the sigverify channel like before.

jbiseda

LGTM

Shreds arriving at tvu/tvu_forward/repair sockets are each processed in a separate thread, and since each thread has its own deduper, the duplicates across these sockets are not filtered out. Using a common deduper across these threads will require an RwLock wrapper and may introduce lock contention. The commit instead moves the shred-deduper to shred-sigverify-stage where all these shreds arrive through the same channel. (cherry picked from commit 25b7811) # Conflicts: # core/src/shred_fetch_stage.rs # core/src/sigverify_shreds.rs

) (#30861) * moves shreds deduper to shred-sigverify stage (#30786) Shreds arriving at tvu/tvu_forward/repair sockets are each processed in a separate thread, and since each thread has its own deduper, the duplicates across these sockets are not filtered out. Using a common deduper across these threads will require an RwLock wrapper and may introduce lock contention. The commit instead moves the shred-deduper to shred-sigverify-stage where all these shreds arrive through the same channel. (cherry picked from commit 25b7811) # Conflicts: # core/src/shred_fetch_stage.rs # core/src/sigverify_shreds.rs * resolves mergify merge conflicts --------- Co-authored-by: behzad nouri <behzadnouri@gmail.com>

behzadnouri mentioned this pull request Mar 18, 2023

removes lazy-static thread-pool from sigverify-shreds #30787

Merged

behzadnouri force-pushed the mv-shred-deduper branch from 2336ac2 to 4ba3396 Compare March 18, 2023 16:06

behzadnouri force-pushed the mv-shred-deduper branch from cbff905 to 88a4c86 Compare March 20, 2023 14:51

behzadnouri force-pushed the mv-shred-deduper branch from 4ba3396 to cbff905 Compare March 20, 2023 14:51

behzadnouri mentioned this pull request Mar 20, 2023

moves turbine-disabled check to shred-fetch-stage #30799

Merged

behzadnouri force-pushed the mv-shred-deduper branch from 88a4c86 to 6baea00 Compare March 20, 2023 17:58

behzadnouri force-pushed the mv-shred-deduper branch from 6baea00 to 86b3b0d Compare March 20, 2023 18:03

behzadnouri force-pushed the mv-shred-deduper branch from 86b3b0d to 6af63ec Compare March 20, 2023 18:23

behzadnouri added a commit that referenced this pull request Mar 20, 2023

removes lazy-static thread-pool from sigverify-shreds (#30787)

c6e7aaf

Instead the thread-pool is passed explicitly from higher in the call stack so that #30786 can use the same thread-pool for shred deduplication.

behzadnouri force-pushed the mv-shred-deduper branch 2 times, most recently from 1e9a73c to 92f2d3b Compare March 20, 2023 21:34

behzadnouri changed the title ~~moves shred-fetch deduper to shred-sigverify stage~~ moves shreds deduper to shred-sigverify stage Mar 20, 2023

behzadnouri force-pushed the mv-shred-deduper branch from 92f2d3b to bc280c3 Compare March 21, 2023 14:53

behzadnouri requested review from t-nelson, sakridge, jbiseda and steviez March 21, 2023 18:48

behzadnouri mentioned this pull request Mar 21, 2023

v1.13: removes lazy-static thread-pool from sigverify-shreds (backport of #30787) #30837

Closed

t-nelson reviewed Mar 21, 2023

View reviewed changes

behzadnouri requested a review from t-nelson March 21, 2023 23:23

t-nelson approved these changes Mar 22, 2023

View reviewed changes

jbiseda approved these changes Mar 22, 2023

View reviewed changes

behzadnouri merged commit 25b7811 into solana-labs:master Mar 22, 2023

behzadnouri deleted the mv-shred-deduper branch March 22, 2023 13:19

behzadnouri added the v1.14 label Mar 22, 2023

mergify bot mentioned this pull request Mar 22, 2023

v1.14: moves shreds deduper to shred-sigverify stage (backport of #30786) #30861

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

moves shreds deduper to shred-sigverify stage #30786

moves shreds deduper to shred-sigverify stage #30786

behzadnouri commented Mar 18, 2023 •

edited

codecov bot commented Mar 21, 2023

t-nelson left a comment

behzadnouri commented Mar 21, 2023

jbiseda left a comment

moves shreds deduper to shred-sigverify stage #30786

moves shreds deduper to shred-sigverify stage #30786

Conversation

behzadnouri commented Mar 18, 2023 • edited

Problem

Summary of Changes

codecov bot commented Mar 21, 2023

Codecov Report

t-nelson left a comment

Choose a reason for hiding this comment

behzadnouri commented Mar 21, 2023

jbiseda left a comment

Choose a reason for hiding this comment

behzadnouri commented Mar 18, 2023 •

edited