perf: db unbundled blocks query optimization #204

segfault-magnet · 2025-03-04T13:07:23Z

closes: #198

Couldn't get the desired performance via indexes for production-level loads.

No choice but to denormalize adding a is_bundled to fuel_blocks.

On the upside makes the queries a lot simpler.

simulated fuel node

…ing' into feat/db_query_optimization

…ization

fuel-cla-bot · 2025-04-03T15:04:44Z

Thanks for the contribution! Before we can merge this, we need @MujkicA to sign the Fuel Labs Contributor License Agreement.

rymnc

reviewed on my phone, more to come (left a marker for me to pick up after)

I think the cla is failing because of a couple commits that need to be reauthored -
e74403b & 70ba25e

packages/adapters/storage/src/lib.rs

rymnc · 2025-04-03T16:27:10Z

packages/adapters/storage/src/postgres.rs

            })
            .collect::<Vec<_>>();

+        sqlx::query!(


hmm, can you move the fragment preparation and query builder before the tx.begin()?

this will help reduce the time spent in the transaction, and therefore reduce waiting elsewhere

packages/adapters/storage/src/postgres.rs

rymnc · 2025-04-03T16:40:25Z

packages/adapters/storage/src/postgres.rs

+}

-    let mut total_bytes = 0;
+async fn take_limited_amount_of_blocks(


Suggested change

async fn take_limited_amount_of_blocks(

async fn take_limited_contiguous_blocks(

rymnc · 2025-04-03T16:45:45Z

packages/adapters/storage/src/postgres.rs

+    let mut contiguous_blocks = Vec::new();
+    let mut cumulative_bytes = 0;
+    let mut total_blocks_count = 0;
    let mut last_height: Option<u32> = None;
-    while let Some(val) = stream.try_next().await? {
-        let data_len = val.data.len();
-        if total_bytes + data_len > max_cumulative_bytes as usize {
+
+    while cumulative_bytes < target_cumulative_bytes {
+        let Some(db_block) = stream.try_next().await? else {
            break;
-        }
+        };

-        let block = CompressedFuelBlock::try_from(val)?;
+        total_blocks_count += 1;
+        let data_len = db_block.data.len() as u32;
+        let block = CompressedFuelBlock::try_from(db_block)?;
        let height = block.height;
-        total_bytes += data_len;

-        blocks.push(block);
-
-        match &mut last_height {
-            Some(last_height) if height != last_height.saturating_add(1) => {
+        // Check if the block is contiguous.
+        if let Some(prev_height) = last_height {
+            if height != prev_height.saturating_add(1) {
+                // A gap is detected. Break out without adding this block.
                break;
            }
-            _ => {
-                last_height = Some(height);
-            }
        }
+        last_height = Some(height);
+        contiguous_blocks.push(block);
+        cumulative_bytes += data_len;
    }

-    Ok(blocks)
+    if cumulative_bytes < target_cumulative_bytes {
+        total_blocks_count += count_remaining_blocks(&mut stream, block_buildup_threshold).await?;
+        let buildup_detected = total_blocks_count >= block_buildup_threshold;
+        Ok((contiguous_blocks, Some(buildup_detected)))
+    } else {
+        // If target bytes are reached without encountering a gap, no buildup detection is needed.
+        Ok((contiguous_blocks, None))
+    }


would something like -

let mut last_height: Option<u32> = None; let mut cumulative_bytes = 0; let mut total_blocks_count = 0; let contiguous_blocks: Vec<CompressedFuelBlock> = stream .try_filter_map(|db_block| async { total_blocks_count += 1; let block = CompressedFuelBlock::try_from(db_block)?; let data_len = block.data.len() as u32; // If there is a height gap, stop processing. if let Some(prev_height) = last_height { if block.height != prev_height.saturating_add(1) { return Ok(None); } } last_height = Some(block.height); cumulative_bytes += data_len; Ok(Some(block)) }) .take_while(|_| std::future::ready(cumulative_bytes < target_cumulative_bytes)) .try_collect() .await?; let buildup_detected = if cumulative_bytes < target_cumulative_bytes { total_blocks_count += count_remaining_blocks(&mut stream, block_buildup_threshold).await?; Some(total_blocks_count >= block_buildup_threshold) } else { None }; Ok((contiguous_blocks, buildup_detected)) }

work?

Two issues:

cumulative_bytes is being borrowed mutably by the try_filter_map and immutably by the take_while causing borrowing issues. Can be worked around by using Arc<AtomicU32>.

try_collect will consume the stream, so I cannot count_remaining_blocks after that.

packages/services/src/block_bundler.rs

xgreenx · 2025-04-06T01:01:10Z

packages/adapters/storage/migrations/0008_add_is_bundled_column.up.sql

+-- 1. Add the column without enforcing NOT NULL initially
+ALTER TABLE fuel_blocks 
+  ADD COLUMN IF NOT EXISTS is_bundled BOOLEAN;


Curios why can't we just set default value to be false from the beginning?

No reason. Will update now.
6ffff3a

xgreenx · 2025-04-06T01:35:35Z

packages/services/src/block_bundler.rs

    pub trait Storage: Sync {
        async fn lowest_sequence_of_unbundled_blocks(
            &self,
            starting_height: u32,


Do we even need starting_height now? It looks like you can sort blocks based by height and is_bundled = false

It's there to give a hard limit to what blocks we no longer care about.

Defensive programming for the unlikely case the committer has not posted for more than 1w.

The starting height is set so that we don't post 1w+ old blocks even if their is_bundled is false.

xgreenx · 2025-04-06T01:38:42Z

packages/services/src/block_bundler.rs

    #[allow(async_fn_in_trait)]
    #[trait_variant::make(Send)]
    pub trait Storage: Sync {
        async fn lowest_sequence_of_unbundled_blocks(


The naming for me was confusing, now when I see how it works, I think that maybe next_candidates_for_bundling, if we remove dependency on starting_height.

I will rename it even though removing starting_height is still being discussed. Candidates is more easily understood, can add a comment on the nature of choosing the candidates.

c53ca9f

forgot the comment:
4c6abd3

xgreenx · 2025-04-06T01:39:24Z

packages/services/src/block_bundler.rs

+            target_cumulative_bytes: u32,
+            block_buildup_detection_threshold: u32,


[nit] It would he nice if you had some BundlingLimits or something like that, where you store all criteria used to limit number of blocks in the bundle.

The Bundler service has a config whose fields impact the number of blocks that are going to end up in a bundle. So all of the relevant limits are found there.

A subsection of that config is given to the storage adapter here to fetch blocks for bundling. The remainder of the bundler config will influence how many of the fetched blocks end up in a bundle.

xgreenx · 2025-04-06T01:52:15Z

helm/fuel-block-committer/templates/deployment.yaml

              path: /health
              port: http
-            initialDelaySeconds: 10
+            initialDelaySeconds: 60


I am curios is there a way to know how long will be migration on the real database from devnet/testnet/mainnet?

xgreenx · 2025-04-06T01:56:21Z

e2e/tests/tests/harness.rs


    let finished = db
-        .lowest_sequence_of_unbundled_blocks(0, 1)
+        .lowest_sequence_of_unbundled_blocks(0, 1, u32::MAX)


[nit] Something like:

Suggested change

.lowest_sequence_of_unbundled_blocks(0, 1, u32::MAX)

.lowest_sequence_of_unbundled_blocks(0, 1, BytesLimit::Unlimited)

Could be more readable=D

packages/adapters/storage/src/postgres.rs

rather than at the end

…dling

segfault-magnet · 2025-04-09T13:15:35Z

I think the cla is failing because of a couple commits that need to be reauthored - e74403b & 70ba25e

I'll reauthor them as soon as all comments are addressed. So that I don't break commit links I've been leaving in replies to the PR comments.

xgreenx · 2025-04-24T23:43:20Z

packages/adapters/storage/src/test_instance.rs

    ) -> services::Result<Option<UnbundledBlocks>> {
        self.db
-            ._lowest_unbundled_blocks(starting_height, max_cumulative_bytes)
+            ._next_candidates_for_bundling(


[nit] In fuel-core, we try not to use _ for internal methods. Instead, we add _inner suffix.

packages/adapters/storage/src/postgres.rs

xgreenx · 2025-04-24T23:52:34Z

packages/adapters/storage/src/postgres.rs

+        // If target bytes are reached without encountering a gap, no buildup detection is needed.
+        Ok((consecutive_blocks, None))


Curious, why is it not needed? It is not clear from the function name and return types=D

segfault-magnet and others added 30 commits February 20, 2025 20:55

bump fuel core

ce1370f

bump fuel core

5af6fbc

remove all that isn't used from the fuel block

dd1c081

latest block via custom query

116868a

block at height via custom query

70bb95e

cleanup

7354012

uncomment

2684357

don't prune prematurely

5f7f6fb

add fuel simulator

e74403b

fix requests

70ba25e

extract binary/docker starting logic into helper along with the

8ae9199

simulated fuel node

start simulator and test

291c06c

simulator works

abe9c6f

can control block size and compressability via FE

0c7142c

metrics are plotted on the control panel

8762dcc

simulated fuel core to run via actix

e45ba16

main server is now actix

b3604a5

cleanup deps

7c20526

remove e2e lib

a2b4fbf

split up fuel core simulated node

e2fc9f0

reformat

2fd8d60

split up bench

86ce42c

remove extra comments

9c374e4

cleanup main

aca924d

add integrity checks for scripts

d9a2434

cleanup

9263c7d

cleanup

9e2931f

add test for simulated node to satisfy fuel adapter

dc4a837

fix CI

dc12cb4

fmt

c0b379e

Merge remote-tracking branch 'origin/bug/schemaspy_orhpan_tables_miss…

a88fbf4

…ing' into feat/db_query_optimization

Base automatically changed from bug/schemaspy_orhpan_tables_missing to master March 13, 2025 11:37

segfault-magnet and others added 3 commits March 13, 2025 14:45

Merge remote-tracking branch 'origin/master' into feat/db_query_optim…

ce20491

…ization

Merge branch 'master' into feat/db_query_optimization

02efed7

Merge branch 'master' into feat/db_query_optimization

db95a59

hal3e requested a review from a team as a code owner April 3, 2025 15:04

fuel-cla-bot bot added the cla:missing label Apr 3, 2025

rymnc reviewed Apr 3, 2025

View reviewed changes

packages/services/src/block_bundler.rs Show resolved Hide resolved

rymnc requested review from a team and removed request for MujkicA, Salka1988 and digorithm April 3, 2025 16:53

xgreenx reviewed Apr 6, 2025

View reviewed changes

segfault-magnet added 7 commits April 9, 2025 09:28

prepare queries before starting a transaction where possible

b117ad1

rename helper

92176ce

make is_bundled false by default at the start of the migration script

6ffff3a

rather than at the end

update sqlx

50b893e

rename lowest_sequence_of_unbundled_blocks to next_candidates_for_bun…

c53ca9f

…dling

add comment

4c6abd3

update sqlx cache for test queries

85415bf

introduce byte limit struct for readability

c06e4eb

segfault-magnet requested review from rymnc and xgreenx April 11, 2025 06:06

rymnc approved these changes Apr 11, 2025

View reviewed changes

xgreenx approved these changes Apr 24, 2025

View reviewed changes

	async fn take_limited_amount_of_blocks(
	async fn take_limited_contiguous_blocks(

		target_cumulative_bytes: u32,
		block_buildup_detection_threshold: u32,

	.lowest_sequence_of_unbundled_blocks(0, 1, u32::MAX)
	.lowest_sequence_of_unbundled_blocks(0, 1, BytesLimit::Unlimited)

		// If target bytes are reached without encountering a gap, no buildup detection is needed.
		Ok((consecutive_blocks, None))

perf: db unbundled blocks query optimization #204

Are you sure you want to change the base?

perf: db unbundled blocks query optimization #204

Uh oh!

Conversation

segfault-magnet commented Mar 4, 2025

Uh oh!

fuel-cla-bot bot commented Apr 3, 2025

Uh oh!

rymnc left a comment • 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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

segfault-magnet Apr 9, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

segfault-magnet commented Apr 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rymnc left a comment •

edited

Loading

segfault-magnet Apr 9, 2025 •

edited

Loading