Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that the sync stream is actually batching work #1167

Closed
freesig opened this issue May 1, 2023 · 1 comment · Fixed by #1318
Closed

Check that the sync stream is actually batching work #1167

freesig opened this issue May 1, 2023 · 1 comment · Fixed by #1318
Assignees

Comments

@freesig
Copy link
Contributor

freesig commented May 1, 2023

While we are executing blocks, we don't request new headers. It means that our flow is: Request 10 headers separately -> request transaction for each available header -> execute 10 blocks -> repeat. It is all done sequentially.
Instead, we should start to request new headers immediately after block execution.

@freesig freesig self-assigned this May 1, 2023
@FuelLabs FuelLabs deleted a comment from freesig May 1, 2023
@xgreenx xgreenx assigned bvrooman and unassigned freesig Jul 10, 2023
@FuelLabs FuelLabs deleted a comment from VuAn1998 Jul 13, 2023
@bvrooman
Copy link
Contributor

bvrooman commented Jul 31, 2023

Locally, I added a test to the back_pressure_tests suite, and added some print statements to output the state each time we get block headers, get transactions, or execute a block. The print statements output the number of headers and transactions in queue at each of these events.

I configured the test with 20 observed heights and the following importer params:

        max_get_header_requests: 10,
        max_get_txns_requests: 10,

The output is as follows:

Incrementing headers:
Headers: 1 - Transactions: 0

Incrementing headers:
Headers: 2 - Transactions: 0

Incrementing headers:
Headers: 3 - Transactions: 0

Incrementing headers:
Headers: 4 - Transactions: 0

Incrementing headers:
Headers: 5 - Transactions: 0

Incrementing headers:
Headers: 6 - Transactions: 0

Incrementing headers:
Headers: 7 - Transactions: 0

Incrementing headers:
Headers: 8 - Transactions: 0

Incrementing headers:
Headers: 9 - Transactions: 0

Incrementing headers:
Headers: 10 - Transactions: 0

Incrementing transactions:
Headers: 0 - Transactions: 1

Incrementing transactions:
Headers: 0 - Transactions: 2

Incrementing transactions:
Headers: 0 - Transactions: 3

Incrementing transactions:
Headers: 0 - Transactions: 4

Incrementing transactions:
Headers: 0 - Transactions: 5

Incrementing transactions:
Headers: 0 - Transactions: 6

Incrementing transactions:
Headers: 0 - Transactions: 7

Incrementing transactions:
Headers: 0 - Transactions: 8

Incrementing transactions:
Headers: 0 - Transactions: 9

Incrementing transactions:
Headers: 0 - Transactions: 10

Incrementing executes:
Headers: 0 - Transactions: 9

Incrementing headers:
Headers: 1 - Transactions: 9

Incrementing headers:
Headers: 2 - Transactions: 9

Incrementing headers:
Headers: 3 - Transactions: 9

Incrementing headers:
Headers: 4 - Transactions: 9

Incrementing headers:
Headers: 5 - Transactions: 9

Incrementing headers:
Headers: 6 - Transactions: 9

Incrementing headers:
Headers: 7 - Transactions: 9

Incrementing headers:
Headers: 8 - Transactions: 9

Incrementing headers:
Headers: 9 - Transactions: 9

Incrementing headers:
Headers: 10 - Transactions: 9

Incrementing executes:
Headers: 10 - Transactions: 8

Incrementing executes:
Headers: 8 - Transactions: 7

Incrementing executes:
Headers: 7 - Transactions: 6

Incrementing executes:
Headers: 6 - Transactions: 5

Incrementing executes:
Headers: 5 - Transactions: 4

Incrementing executes:
Headers: 4 - Transactions: 3

Incrementing executes:
Headers: 3 - Transactions: 2

Incrementing executes:
Headers: 2 - Transactions: 1

Incrementing executes:
Headers: 1 - Transactions: 0

Incrementing transactions:
Headers: 0 - Transactions: 1

Incrementing transactions:
Headers: 0 - Transactions: 2

Incrementing transactions:
Headers: 0 - Transactions: 3

Incrementing transactions:
Headers: 0 - Transactions: 4

Incrementing transactions:
Headers: 0 - Transactions: 5

Incrementing transactions:
Headers: 0 - Transactions: 6

Incrementing transactions:
Headers: 0 - Transactions: 7

Incrementing transactions:
Headers: 0 - Transactions: 8

Incrementing transactions:
Headers: 0 - Transactions: 9

Incrementing transactions:
Headers: 0 - Transactions: 10

Incrementing executes:
Headers: 0 - Transactions: 9

Incrementing headers:
Headers: 1 - Transactions: 9

Incrementing executes:
Headers: 1 - Transactions: 8

Incrementing executes:
Headers: 0 - Transactions: 7

Incrementing executes:
Headers: 0 - Transactions: 6

Incrementing executes:
Headers: 0 - Transactions: 5

Incrementing executes:
Headers: 0 - Transactions: 4

Incrementing executes:
Headers: 0 - Transactions: 3

Incrementing executes:
Headers: 0 - Transactions: 2

Incrementing executes:
Headers: 0 - Transactions: 1

Incrementing executes:
Headers: 0 - Transactions: 0

Incrementing transactions:
Headers: 0 - Transactions: 1

Incrementing executes:
Headers: 0 - Transactions: 0

From this test it appears that there is some sequential processing and that the buffers become lopsided first towards headers, and then towards transactions:

Incrementing headers:
Headers: 1 - Transactions: 0

...

Incrementing headers:
Headers: 10 - Transactions: 0

Incrementing transactions:
Headers: 0 - Transactions: 1

...

Incrementing transactions:
Headers: 0 - Transactions: 10

The maximum values here correspond with the arguments provided for the maximum values in the import config.

It appears that it first saturates the buffer with the maximum number of headers, then it saturates the buffer with the maximum number of transactions. This behaviour is consistent whenever I run the test, and with any configuration of maximum headers and maximum transactions.

bvrooman pushed a commit that referenced this issue Aug 24, 2023
Related issues:
 - #1167

This PR adds benchmarking for block synchronization. 

Benchmarks simulate the import by using mock services that introduce
artificial delays. Benchmarks are provisioned by reusing existing test
types and mock ports. This PR refactors the existing back pressure tests
to extract its test structures so that they can be reused in benchmarks.

In a follow up PR, I will be refactoring the block import to use a
single buffer and asynchronous tasks to perform header downloads.

---------

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
bvrooman pushed a commit that referenced this issue Aug 28, 2023
Related issues:
- Closes #1167

---------

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: xgreenx <xgreenx9999@gmail.com>
crypto523 pushed a commit to crypto523/fuel-core that referenced this issue Oct 7, 2024
Related issues:
 - FuelLabs/fuel-core#1167

This PR adds benchmarking for block synchronization. 

Benchmarks simulate the import by using mock services that introduce
artificial delays. Benchmarks are provisioned by reusing existing test
types and mock ports. This PR refactors the existing back pressure tests
to extract its test structures so that they can be reused in benchmarks.

In a follow up PR, I will be refactoring the block import to use a
single buffer and asynchronous tasks to perform header downloads.

---------

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
crypto523 pushed a commit to crypto523/fuel-core that referenced this issue Oct 7, 2024
Related issues:
- Closes FuelLabs/fuel-core#1167

---------

Co-authored-by: Brandon Kite <brandonkite92@gmail.com>
Co-authored-by: xgreenx <xgreenx9999@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants