retransmits shreds recovered from erasure codes #19233

behzadnouri · 2021-08-13T20:16:18Z

Problem

Shreds recovered from erasure codes have not been received from turbine
and have not been retransmitted to other nodes downstream. This results
in more repairs across the cluster which is slower.

Summary of Changes

4th commit reworks retransmit stage to simply receive shreds instead of packets; because converting recovered "shreds" to "packets" just to send them to retransmit-stage is superfluous. This will also avoid partially deserializing packet in retransmit stage when the entire packet was already deserialized to shred earlier in the window-stage.
In preparation of the above change, first 2 commits move a number of metrics related to packet-counts from retransmit stage to window-service earlier in the pipeline. Metrics for repair and discard packets are emitted in the window stage and, with the 4th commit, those packets are no longer sent down to retransmit-stage (which was pointless since they will not be retransmitted).
Last commit, channels through recovered shreds to retransmit stage in order to further broadcast the shreds to downstream nodes in the tree.

codecov · 2021-08-14T18:12:36Z

Codecov Report

Merging #19233 (8f5553c) into master (692aa99) will decrease coverage by 0.0%.
The diff coverage is 79.9%.

@@            Coverage Diff            @@
##           master   #19233     +/-   ##
=========================================
- Coverage    82.8%    82.8%   -0.1%     
=========================================
  Files         455      455             
  Lines      130044   129960     -84     
=========================================
- Hits       107780   107692     -88     
- Misses      22264    22268      +4

Working towards sending shreds (instead of packets) to retransmit stage so that shreds recovered from erasure codes are as well retransmitted. Following commit will add these metrics back to window-service, earlier in the pipeline.

Adding back these metrics from the earlier commit which removed them from retransmit stage.

Working towards channelling through shreds recovered from erasure codes to retransmit stage.

instead of opaque (u32, u32) which are then converted to CompletedDataSetInfo at the call-site.

Shreds recovered from erasure codes have not been received from turbine and have not been retransmitted to other nodes downstream. This results in more repairs across the cluster which is slower. This commit channels through recovered shreds to retransmit stage in order to further broadcast the shreds to downstream nodes in the tree.

carllin · 2021-08-17T05:45:34Z

core/src/window_service.rs

+            .zip(&repair_infos)
+            .filter(|(_, repair_info)| repair_info.is_none())
+            .map(|(shred, _)| shred)
+            .cloned()


Wonder if we could avoid costly clones if we switched over to some Arc<Shred>, but that's a story for another time

carllin · 2021-08-17T05:47:53Z

core/src/retransmit_stage.rs

        rpc_subscriptions: Option<Arc<RpcSubscriptions>>,
        duplicate_slots_sender: Sender<Slot>,
        ancestor_hashes_replay_update_receiver: AncestorHashesReplayUpdateReceiver,
    ) -> Self {
        let (retransmit_sender, retransmit_receiver) = channel();
+        // https://github.com/rust-lang/rust/issues/39364#issuecomment-634545136
+        let _retransmit_sender = retransmit_sender.clone();


ick, I've run into this too, was annoying to debug

carllin · 2021-08-17T06:00:19Z

Shreds recovered from erasure codes have not been received from turbine
and have not been retransmitted to other nodes downstream. This results
in more repairs across the cluster which is slower.

Hmm, what if in cases where we just happen to receive some combination of erasure shreds + data shreds first such that we recover some data shreds that we eventually would have gotten from turbine. Will this cause a massive spike in bandwidth if we then retransmit these recovered shreds, even though turbine had no problem circulating them?

To avoid this, should we wait for a bit to see that these recovered shreds don't ultimately arrive through turbine?

carllin

Nice, this is so much cleaner, awesome! 😃

jbiseda

looks good!

jbiseda · 2021-08-17T06:41:02Z

core/src/window_service.rs

        bank_forks: Arc<RwLock<BankForks>>,
-        retransmit: PacketSender,
+        retransmit_sender: Sender<Vec<Shred>>,


thanks for renaming these :)

behzadnouri · 2021-08-17T12:59:48Z

Hmm, what if in cases where we just happen to receive some combination of erasure shreds + data shreds first such that we recover some data shreds that we eventually would have gotten from turbine. Will this cause a massive spike in bandwidth if we then retransmit these recovered shreds, even though turbine had no problem circulating them?

To avoid this, should we wait for a bit to see that these recovered shreds don't ultimately arrive through turbine?

That should be picked up by duplicates check in retransmit stage (i.e. check_if_already_received) and so the 2nd copy of the shred will be skipped:
https://github.com/solana-labs/solana/blob/0e5ea36cc/core/src/retransmit_stage.rs#L227-L253

…0249) * removes packet-count metrics from retransmit stage Working towards sending shreds (instead of packets) to retransmit stage so that shreds recovered from erasure codes are as well retransmitted. Following commit will add these metrics back to window-service, earlier in the pipeline. (cherry picked from commit bf437b0) # Conflicts: # core/src/retransmit_stage.rs * adds packet/shred count stats to window-service Adding back these metrics from the earlier commit which removed them from retransmit stage. (cherry picked from commit 8198a7e) * removes erroneous uses of Arc<...> from retransmit stage (cherry picked from commit 6e41333) # Conflicts: # core/src/retransmit_stage.rs # core/src/tvu.rs * sends shreds (instead of packets) to retransmit stage Working towards channelling through shreds recovered from erasure codes to retransmit stage. (cherry picked from commit 3efccbf) # Conflicts: # core/src/retransmit_stage.rs * returns completed-data-set-info from insert_data_shred instead of opaque (u32, u32) which are then converted to CompletedDataSetInfo at the call-site. (cherry picked from commit 3c71670) # Conflicts: # ledger/src/blockstore.rs * retransmits shreds recovered from erasure codes Shreds recovered from erasure codes have not been received from turbine and have not been retransmitted to other nodes downstream. This results in more repairs across the cluster which is slower. This commit channels through recovered shreds to retransmit stage in order to further broadcast the shreds to downstream nodes in the tree. (cherry picked from commit 7a8807b) # Conflicts: # core/src/retransmit_stage.rs # core/src/window_service.rs * removes backport merge conflicts Co-authored-by: behzad nouri <behzadnouri@gmail.com>

test_skip_repair in retransmit-stage is no longer relevant because following: solana-labs#19233 repair packets are filtered out earlier in window-service and so retransmit stage does not know if a shred is repaired or not. Also, following turbine peer shuffle changes: solana-labs#24080 the test has become flaky since it does not take into account how peers are shuffled for each shred.

…4121) test_skip_repair in retransmit-stage is no longer relevant because following: #19233 repair packets are filtered out earlier in window-service and so retransmit stage does not know if a shred is repaired or not. Also, following turbine peer shuffle changes: #24080 the test has become flaky since it does not take into account how peers are shuffled for each shred.

…4121) test_skip_repair in retransmit-stage is no longer relevant because following: #19233 repair packets are filtered out earlier in window-service and so retransmit stage does not know if a shred is repaired or not. Also, following turbine peer shuffle changes: #24080 the test has become flaky since it does not take into account how peers are shuffled for each shred. (cherry picked from commit 2282571)

…4121) (#24126) test_skip_repair in retransmit-stage is no longer relevant because following: #19233 repair packets are filtered out earlier in window-service and so retransmit stage does not know if a shred is repaired or not. Also, following turbine peer shuffle changes: #24080 the test has become flaky since it does not take into account how peers are shuffled for each shred. (cherry picked from commit 2282571) Co-authored-by: behzad nouri <behzadnouri@gmail.com>

…4121) test_skip_repair in retransmit-stage is no longer relevant because following: #19233 repair packets are filtered out earlier in window-service and so retransmit stage does not know if a shred is repaired or not. Also, following turbine peer shuffle changes: #24080 the test has become flaky since it does not take into account how peers are shuffled for each shred. (cherry picked from commit 2282571) # Conflicts: # core/src/retransmit_stage.rs

…ckport #24121) (#24663) * removes outdated and flaky test_skip_repair from retransmit-stage (#24121) test_skip_repair in retransmit-stage is no longer relevant because following: #19233 repair packets are filtered out earlier in window-service and so retransmit stage does not know if a shred is repaired or not. Also, following turbine peer shuffle changes: #24080 the test has become flaky since it does not take into account how peers are shuffled for each shred. (cherry picked from commit 2282571) # Conflicts: # core/src/retransmit_stage.rs * removes mergify merge conflicts Co-authored-by: behzad nouri <behzadnouri@gmail.com>

behzadnouri changed the title ~~retransmits shreds recovered from erasure codings~~ retransmits shreds recovered from erasure codes Aug 13, 2021

behzadnouri force-pushed the retrans-shreds branch 3 times, most recently from 12b60d2 to 0de4fc8 Compare August 14, 2021 16:26

behzadnouri requested review from sakridge, jbiseda and carllin August 14, 2021 18:14

behzadnouri force-pushed the retrans-shreds branch from 0de4fc8 to 4e06f9c Compare August 15, 2021 17:57

behzadnouri added 6 commits August 16, 2021 15:20

removes packet-count metrics from retransmit stage

4a434d9

Working towards sending shreds (instead of packets) to retransmit stage so that shreds recovered from erasure codes are as well retransmitted. Following commit will add these metrics back to window-service, earlier in the pipeline.

adds packet/shred count stats to window-service

3d1c5a8

Adding back these metrics from the earlier commit which removed them from retransmit stage.

removes erroneous uses of Arc<...> from retransmit stage

a6d2472

sends shreds (instead of packets) to retransmit stage

507367e

Working towards channelling through shreds recovered from erasure codes to retransmit stage.

returns completed-data-set-info from insert_data_shred

ff862bb

instead of opaque (u32, u32) which are then converted to CompletedDataSetInfo at the call-site.

behzadnouri force-pushed the retrans-shreds branch from 4e06f9c to 8f5553c Compare August 16, 2021 19:27

carllin reviewed Aug 17, 2021

View reviewed changes

carllin approved these changes Aug 17, 2021

View reviewed changes

jbiseda approved these changes Aug 17, 2021

View reviewed changes

KeKo6988 approved these changes Aug 17, 2021

View reviewed changes

solana-labs deleted a comment from KeKo6988 Aug 17, 2021

behzadnouri merged commit 7a8807b into solana-labs:master Aug 17, 2021

behzadnouri deleted the retrans-shreds branch August 17, 2021 13:44

behzadnouri mentioned this pull request Aug 17, 2021

Retransmit recovered shreds through turbine #6594

Closed

behzadnouri added the v1.7 label Sep 27, 2021

mergify bot mentioned this pull request Sep 27, 2021

retransmits shreds recovered from erasure codes (backport #19233) #20249

Merged

behzadnouri mentioned this pull request Apr 5, 2022

removes outdated and flaky test_skip_repair from retransmit-stage #24121

Merged

behzadnouri mentioned this pull request Apr 14, 2022

validator uses more memory than expected when metrics are enabled #24319

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

retransmits shreds recovered from erasure codes #19233

retransmits shreds recovered from erasure codes #19233

behzadnouri commented Aug 13, 2021 •

edited

Loading

codecov bot commented Aug 14, 2021 •

edited

Loading

carllin Aug 17, 2021 •

edited

Loading

carllin Aug 17, 2021 •

edited

Loading

carllin commented Aug 17, 2021

carllin left a comment

jbiseda left a comment

jbiseda Aug 17, 2021

behzadnouri commented Aug 17, 2021

retransmits shreds recovered from erasure codes #19233

retransmits shreds recovered from erasure codes #19233

Conversation

behzadnouri commented Aug 13, 2021 • edited Loading

Problem

Summary of Changes

codecov bot commented Aug 14, 2021 • edited Loading

Codecov Report

carllin Aug 17, 2021 • edited Loading

Choose a reason for hiding this comment

carllin Aug 17, 2021 • edited Loading

Choose a reason for hiding this comment

carllin commented Aug 17, 2021

carllin left a comment

Choose a reason for hiding this comment

jbiseda left a comment

Choose a reason for hiding this comment

jbiseda Aug 17, 2021

Choose a reason for hiding this comment

behzadnouri commented Aug 17, 2021

behzadnouri commented Aug 13, 2021 •

edited

Loading

codecov bot commented Aug 14, 2021 •

edited

Loading

carllin Aug 17, 2021 •

edited

Loading

carllin Aug 17, 2021 •

edited

Loading