Pipeline broadcast socket transmit and blocktree record by aeyakovenko · Pull Request #7481 · solana-labs/solana

aeyakovenko · 2019-12-14T05:37:28Z

Problem

Broadcast stage does a sequential

generate shreds (a bunch of steps)
send data shreds
record blocktree shrds
send coding shreds

Summary of Changes

split out record and send Into their own threads. Sockets, blocktree, signing touch 3 different pieces of hardware.

new pipeline looks like

1. generate data shreds
2. async send data shreds
3. async record data shreds
4. generate coding shreds
5. async send coding shreds

Broadcast spikes are gone!!! Confirmation time is 40% better. Seems like we lost 7% of average perf, but peaks are 20% higher. Seems like a win. I am also seeing 200+ shreds to sign, so it might start making sense to go to the GPU.

Before:

After:

Fixes #

aeyakovenko · 2019-12-14T17:49:27Z

@mvines i added multiple broadcast sockets, and now i see this:

panicked at 'assertion failed: udp_ports.len() <= msg.udp_ports.len()', net-utils/src/ip_echo_server.rs:26:9
--

sagar-solana · 2019-12-16T19:05:35Z

@aeyakovenko Record and Transmit should be strictly ordered right? Otherwise we run the risk of transmitting something we didn't record. It can cause the leader to make double transmissions (and get slashed) if it crashes after tx but before record.

codecov · 2019-12-16T19:08:12Z

Codecov Report

Merging #7481 into master will increase coverage by 14.9%.
The diff coverage is 73.2%.

@@            Coverage Diff            @@
##           master   #7481      +/-   ##
=========================================
+ Coverage    65.8%   80.7%   +14.9%     
=========================================
  Files         245     244       -1     
  Lines       60342   48911   -11431     
=========================================
- Hits        39718   39512     -206     
+ Misses      20624    9399   -11225

aeyakovenko · 2019-12-16T19:09:12Z

@sagar-solana
Leaders shouldn't try to generate the same block. If they reset, they should wait until the next block.

sagar-solana · 2019-12-16T19:11:05Z

@sagar-solana
Leaders shouldn't try to generate the same block. If they reset, they should wait until the next block.

How would they know that they've already generated this block? Right now they use blocktree as a signal to determine whether or not a block was made.

aeyakovenko · 2019-12-16T19:21:21Z

@sagar-solana the validator boot sequence forces them to wait for a live tx to be acknowledged by the network, which means that they have to skip their half built block anyways.

sagar-solana · 2019-12-16T19:26:40Z

@aeyakovenko cool, that should avoid this. Has that already been merged? We'll have to make sure that's done before slashing comes in.

aeyakovenko · 2019-12-16T20:48:45Z

@carllin how is that different if today we corrupted rocks on reset? There is no guarantee of data availability.

Pull request has been modified.

aeyakovenko · 2019-12-16T21:02:32Z

+        } else {
+            socket_sender.send((stakes.clone(), data_shreds.clone()))?;
+            blocktree_sender.send(data_shreds.clone())?;
+        }


@sagar-solana @carllin wdyt?

pgarg66 · 2019-12-16T21:48:06Z

+        //Insert the first shred so blocktree stores that the leader started this block
+        //This must be done before the blocks are sent out over the wire.
+        if data_shreds.len() > 0 && data_shreds[0].index() == 0 {
+            let first = vec![data_shreds[0].clone()];


Wouldn't this cause the first shred to be inserted twice? First time here, and then thru blocktree_sender.send()? Even though, the second insert will eventually fail.

I thnk right now b/c the is_trusted flag iis set, it will jsut overwrite it the seccond tiime XD, it's ok, shreds are small :D

Sounds like a problem waiting to happen..

@pgarg66 why? I would expect the database to be sound if the same data is inserted twice. that seems like a simple requirement.

I think it's less about whether the db can handle it and more about a redundant call to insert the same data. Nbd I think since we'll hopefully get rid of this later.

@sagar-solana what is the concern? I can't imagine this ever being noticeable for performance.

solana-grimes · 2019-12-16T21:50:32Z

💔 Unable to automerge due to CI failure

Pull request has been modified.

solana-grimes · 2019-12-16T22:36:49Z

💔 Unable to automerge due to CI failure

…serts of that shred

StandardBroadcastRun::insert skips 1st shred with index zero because the 1st *data* shred is inserted synchronously: https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 solana-labs#7481 which added this code was not inserting coding shreds into blockstore. Starting with solana-labs#8899 coding shreds are inserted into blockstore as well as data shreds, but the insert logic erroneously skips first coding shred because it does not check if shred is code or data.

#25916) StandardBroadcastRun::insert skips 1st shred with index zero because the 1st *data* shred is inserted synchronously: https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 #7481 which added this code was not inserting coding shreds into blockstore. Starting with #8899 coding shreds are inserted into blockstore as well as data shreds, but the insert logic erroneously skips first coding shred because it does not check if shred is code or data.

#25916) StandardBroadcastRun::insert skips 1st shred with index zero because the 1st *data* shred is inserted synchronously: https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 #7481 which added this code was not inserting coding shreds into blockstore. Starting with #8899 coding shreds are inserted into blockstore as well as data shreds, but the insert logic erroneously skips first coding shred because it does not check if shred is code or data. (cherry picked from commit eacb918)

#25916) (#26005) StandardBroadcastRun::insert skips 1st shred with index zero because the 1st *data* shred is inserted synchronously: https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 #7481 which added this code was not inserting coding shreds into blockstore. Starting with #8899 coding shreds are inserted into blockstore as well as data shreds, but the insert logic erroneously skips first coding shred because it does not check if shred is code or data. (cherry picked from commit eacb918) Co-authored-by: behzad nouri <behzadnouri@gmail.com>

#25916) (#26006) StandardBroadcastRun::insert skips 1st shred with index zero because the 1st *data* shred is inserted synchronously: https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L239-L246 https://github.com/solana-labs/solana/blob/53695ecd2/core/src/broadcast_stage/standard_broadcast_run.rs#L334-L339 #7481 which added this code was not inserting coding shreds into blockstore. Starting with #8899 coding shreds are inserted into blockstore as well as data shreds, but the insert logic erroneously skips first coding shred because it does not check if shred is code or data. (cherry picked from commit eacb918) Co-authored-by: behzad nouri <behzadnouri@gmail.com>

aeyakovenko added 6 commits December 13, 2019 21:35

wip

f1998aa

wip

d2f0ac2

wip

784c87d

wip

306ec46

update

a6b0cdd

checks

0215540

aeyakovenko requested review from pgarg66 and sakridge December 14, 2019 09:10

clippy

ad3b34e

aeyakovenko marked this pull request as ready for review December 14, 2019 09:17

aeyakovenko added 3 commits December 14, 2019 01:35

clippy

965d87c

generrate coding and data shreds in different stages

df65613

fixup! generrate coding and data shreds in different stages

c1ee3a1

aeyakovenko requested a review from sagar-solana December 14, 2019 17:54

aeyakovenko added 8 commits December 14, 2019 10:00

ip echo server needs more ports

330b281

@mvines no clue why this lets colo start

334427d

tests

d5f216d

fixup! tests

a1b93fd

fixup! fixup! tests

a0fef25

fixup! fixup! fixup! tests

063b02d

fixup! fixup! fixup! fixup! tests

6a93954

fixup! fixup! fixup! fixup! fixup! tests

d719c25

@mvines maybe this works?

7f5aa0b

Store the first shred in blocktree

e4d38cb

aeyakovenko commented Dec 16, 2019

View reviewed changes

carllin previously approved these changes Dec 16, 2019

View reviewed changes

pgarg66 reviewed Dec 16, 2019

View reviewed changes

aeyakovenko added the automerge Merge this Pull Request automatically once CI passes label Dec 16, 2019

solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Dec 16, 2019

aeyakovenko added the automerge Merge this Pull Request automatically once CI passes label Dec 16, 2019

solana-grimes removed the automerge Merge this Pull Request automatically once CI passes label Dec 16, 2019

aeyakovenko force-pushed the broadcast_socket_thread branch from dba7d97 to e4d38cb Compare December 16, 2019 23:39

only insert the first shred synchronoulsy; guard against duplicate in…

0049703

…serts of that shred

aeyakovenko force-pushed the broadcast_socket_thread branch from 1a861ea to 0049703 Compare December 17, 2019 00:09

aeyakovenko added the automerge Merge this Pull Request automatically once CI passes label Dec 17, 2019

solana-grimes merged commit 97589f7 into solana-labs:master Dec 17, 2019

behzadnouri mentioned this pull request Jun 11, 2022

patches bug where the 1st coding shred is not inserted into blockstore #25916

Merged

Conversation

aeyakovenko commented Dec 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

aeyakovenko commented Dec 14, 2019

Uh oh!

sagar-solana commented Dec 16, 2019

Uh oh!

codecov Bot commented Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aeyakovenko commented Dec 16, 2019

Uh oh!

sagar-solana commented Dec 16, 2019

Uh oh!

aeyakovenko commented Dec 16, 2019

Uh oh!

sagar-solana commented Dec 16, 2019

Uh oh!

aeyakovenko commented Dec 16, 2019

Uh oh!

aeyakovenko Dec 16, 2019

Choose a reason for hiding this comment

Uh oh!

pgarg66 Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

carllin Dec 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pgarg66 Dec 16, 2019

Choose a reason for hiding this comment

Uh oh!

aeyakovenko Dec 16, 2019

Choose a reason for hiding this comment

Uh oh!

sagar-solana Dec 16, 2019

Choose a reason for hiding this comment

Uh oh!

aeyakovenko Dec 16, 2019

Choose a reason for hiding this comment

Uh oh!

solana-grimes commented Dec 16, 2019

Uh oh!

solana-grimes commented Dec 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

aeyakovenko commented Dec 14, 2019 •

edited

Loading

codecov Bot commented Dec 16, 2019 •

edited

Loading

pgarg66 Dec 16, 2019 •

edited

Loading

carllin Dec 16, 2019 •

edited

Loading