[moul]

BlockStore.SaveBlock flushes to disk twice per block: once inside BlockStoreStateJSON.Save (a SetSync), then again with the trailing bs.db.SetSync(nil, nil) "Flush", which writes an empty key purely to force a second fsync. The first flush already made every earlier write durable (pebble and goleveldb sync one ordered WAL), so the second one buys nothing and costs a full fsync on every height.

This PR stages the whole block (meta, parts, both commits, the height descriptor) in one batch and commits it with a single WriteSync. The save also becomes atomic: a crash can no longer leave a height whose meta is on disk without its parts. BlockStoreStateJSON.Save keeps its signature and behaviour for other callers.

Why it matters

Fast sync applies blocks strictly one at a time, and per block it does five synced writes: SaveBlock (x2), SaveABCIResponses, the app Commit, and SaveState. A Go execution trace of a node replaying gnoland-1 from genesis shows the replay goroutine blocked on those pebble commits for 6.86 s of an 8 s window, split almost evenly across the five call sites, while the process used ~18% of one core. On ordinary hardware, catching up is fsync-latency bound, not CPU bound, so removing one of the five flushes is close to a 20% speedup.

Measured

Replaying gnoland-1 from genesis with v1.5.0 (same data dir, swapping the binary between rounds, 45 s warmup then 90 s measured, rounds interleaved), pebble backend, Apple Silicon laptop SSD:

round	heights (base)	base blocks/s	heights (patched)	patched blocks/s
1	48161..52751	51.0	55214..60872	62.9
2	62976..67633	51.7	69897..75438	61.6
3	77814..82184	48.6	84431..89693	58.5
4	91231..96324	56.6	99112..105382	69.7
mean		52.0		63.2 (+21.5%)

Disks with slower flushes (SATA, RAID1, no power-loss protection) pay more per fsync, so they gain at least as much. The same path runs once per block in consensus too.

Safety

• Ordering against the other stores is unchanged: the block is durable before ApplyBlock runs, exactly as before.
• Every backend implements Batch.WriteSync. For boltdb, where each Set was its own fsynced transaction, this goes from six-plus commits per block to one.
• The contiguity check in the removed saveBlockPart duplicated the one at the top of SaveBlock, which still runs before anything is staged.
• No on-chain or consensus-visible change; only the node's local block store write pattern.

Test

TestSaveBlockFlushesOnce wraps a memdb and counts synced writes (direct SetSync/DeleteSync and batch WriteSync). It fails on master (expected: 1, actual: 2) and passes here. go test ./tm2/pkg/bft/{store,blockchain,state,node,consensus}/... is green.

Not in this PR

The remaining four flushes are each load-bearing for crash recovery (the ABCI responses must be durable before the app commits; state after it). Further gains need structural changes, for example pipelining the block-store write of height N+1 with the execution of N, or making the per-block auth params read (EndBlocker decodes unrestricted_addrs from JSON every block, ~16% of replay CPU at low heights) cheaper.

🤖 Generated with Claude Code

[Gno2D2]

🛠 PR Checks Summary

All Automated Checks passed. ✅

Manual Checks (for Reviewers):

• IGNORE the bot requirements for this PR (force green CI check)

Read More

🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers.

✅ Automated Checks (for Contributors):

No automated checks match this pull request.

☑️ Contributor Actions:

1. Fix any issues flagged by automated checks.
2. Follow the Contributor Checklist to ensure your PR is ready for review.
   • Add new tests, or document why they are unnecessary.
   • Provide clear examples/screenshots, if necessary.
   • Update documentation, if required.
   • Ensure no breaking changes, or include BREAKING CHANGE notes.
   • Link related issues/PRs, where applicable.

☑️ Reviewer Actions:

1. Complete manual checks for the PR, including the guidelines and additional checks if applicable.

📚 Resources:

• Report a bug with the bot.
• View the bot’s configuration file.

Debug

Manual Checks

**IGNORE** the bot requirements for this PR (force green CI check)

If

🟢 Condition met
└── 🟢 On every pull request

Can be checked by

• Any user with comment edit permission

[moul]

Rebased onto master at db74a1124 → head a280399c6.

Conflict: one, in tm2/pkg/bft/store/store.go BlockStoreStateJSON.Save, against #6334 (deferred block-sync writes), which made Save panic when its SetSync fails.

• Resolution: kept both. Save still does its own SetSync and panics on error, as perf(tm2): defer block sync's writes to ordered flush points #6334 wants. It now gets its bytes from this PR's bytes() helper, which SaveBlock uses to stage the height record in the batch.
• Composition with perf(tm2): defer block sync's writes to ordered flush points #6334: during block sync bs.db is a HoldingDB. While holding, NewBatch() returns the collector's batchHandle, so WriteSync moves the staged ops into the held writes and does not touch the disk. They are written at the flush point, as perf(tm2): defer block sync's writes to ordered flush points #6334 intends. Outside holding (consensus, or an app that does not defer), this PR's single fsync applies as before. I updated the SaveBlock comment to say this.
• Effect on the measured gain: perf(tm2): defer block sync's writes to ordered flush points #6334 already takes these flushes out of gno.land's block sync. Its body says perf(tm2): save a block with one fsync instead of two #6264 still applies in consensus mode. The +21.5% replay number above was measured before perf(tm2): defer block sync's writes to ordered flush points #6334, and I have not re-measured it on top.

Gate (lyra, nice -n 10, GOTOOLCHAIN=auto):

$ cd tm2 && go build ./...                      # OK
$ go vet ./pkg/bft/store/                       # OK
$ go test -run TestSaveBlockFlushesOnce ./pkg/bft/store/
--- PASS: TestSaveBlockFlushesOnce
# same test against master's store.go (branch test file, master impl):
--- FAIL: TestSaveBlockFlushesOnce
        	expected: 1
        	actual  : 2
$ go test -count=1 ./pkg/bft/{store,blockchain,state,node,consensus}/... ./pkg/db/
ok  tm2/pkg/bft/store        0.043s
ok  tm2/pkg/bft/blockchain   8.578s
ok  tm2/pkg/bft/state        0.490s   (+ eventstore/... ok)
ok  tm2/pkg/bft/node        10.475s
ok  tm2/pkg/bft/consensus   60.684s   (includes TestHandshakeAfterFlushPointPowerCut)
ok  tm2/pkg/db               0.174s

[thehowl]

[note: ai-assisted]

Severity: 2/10, low

Nit: the description's +21.5% block-sync speedup was measured on v1.5.0 and predates #6334. On master, block sync holds SaveBlock's writes until a flush point, so a block costs 0 fsyncs there with or without this PR. The gain left is the atomic save, plus one fsync per block on a live node, which #6316's resync timed at 2.21 → 1.23 ms per SaveBlock.

fsyncs per SaveBlock, master vs this PR

tm2/pkg/bft/store/zz_review_test.go, with the block store wired as in node.go, a HoldingDB over the disk:

package store

import (
	"testing"

	"github.com/gnolang/gno/tm2/pkg/bft/types"
	tmtime "github.com/gnolang/gno/tm2/pkg/bft/types/time"
	dbm "github.com/gnolang/gno/tm2/pkg/db"
	"github.com/gnolang/gno/tm2/pkg/db/memdb"
	"github.com/gnolang/gno/tm2/pkg/log"
)

// revCountDB counts writes that reach the disk with a sync.
type revCountDB struct {
	dbm.DB
	syncs int
}

func (db *revCountDB) SetSync(k, v []byte) error { db.syncs++; return db.DB.SetSync(k, v) }
func (db *revCountDB) DeleteSync(k []byte) error { db.syncs++; return db.DB.DeleteSync(k) }
func (db *revCountDB) NewBatch() dbm.Batch       { return &revCountBatch{db.DB.NewBatch(), db} }
func (db *revCountDB) NewBatchWithSize(n int) dbm.Batch {
	return &revCountBatch{db.DB.NewBatchWithSize(n), db}
}

type revCountBatch struct {
	dbm.Batch
	db *revCountDB
}

func (b *revCountBatch) WriteSync() error { b.db.syncs++; return b.Batch.WriteSync() }

// Mirrors node.go's wiring: the block store's DB is a HoldingDB over the disk.
func TestReviewSaveBlockSyncs(t *testing.T) {
	state, _, cleanup := makeStateAndBlockStore(log.NewNoopLogger())
	defer cleanup()

	disk := &revCountDB{DB: memdb.NewMemDB()}
	h := dbm.NewHoldingDB(disk)
	bs := NewBlockStore(h)

	save := func(height int64) {
		b := makeBlock(height, state, new(types.Commit))
		bs.SaveBlock(b, b.MakePartSet(2), makeTestCommit(height, tmtime.Now()))
	}

	// Live node (consensus): writes pass through.
	save(1)
	t.Logf("not held (consensus): syncs for 1 SaveBlock = %d", disk.syncs)

	// Block sync since #6334: writes held until a flush point.
	disk.syncs = 0
	h.Hold(true)
	for height := int64(2); height <= 11; height++ {
		save(height)
	}
	t.Logf("held (block sync): syncs for 10 SaveBlocks = %d", disk.syncs)
	if err := h.WriteHeld(); err != nil {
		t.Fatal(err)
	}
	h.Hold(false)
	t.Logf("held (block sync): syncs after the flush point = %d", disk.syncs)

	empty, _ := disk.DB.Has([]byte{})
	t.Logf("empty key on disk: %v; reopened height: %d", empty, NewBlockStore(disk.DB).Height())
}

Run: go test ./tm2/pkg/bft/store/ -run 'TestReviewSaveBlockSyncs$' -v -count=1

	master db74a11249	this PR a280399c65
live node, 1 SaveBlock	2	1
block sync, 10 held SaveBlocks	0	0
block sync, at the flush point	1	1