Refactor changelog to generic WAL #2671

yzang2019 · 2026-01-07T06:56:20Z

Describe your changes and provide context

Purpose of this PR:

Make WAL generic type, previously it is only able to store changeset. Make changelog a common implementation of WAL[ChangeLogEntry[
Bump version of tidywal to latest
Refactor MemIAVL to use the new interface

Testing performed to validate your change

Added and modified unit test

github-actions · 2026-01-07T06:57:05Z

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
`✅ passed`	`✅ passed`	`✅ passed`	`✅ passed`	Jan 16, 2026, 2:17 AM

codecov · 2026-01-07T07:01:47Z

Codecov Report

❌ Patch coverage is 65.49708% with 118 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.76%. Comparing base (c3b387c) to head (784f7c2).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
sei-db/wal/wal.go	66.06%	36 Missing and 20 partials ⚠️
sei-db/state_db/sc/memiavl/db.go	62.22%	21 Missing and 13 partials ⚠️
sei-db/wal/changelog.go	0.00%	8 Missing ⚠️
sei-db/state_db/sc/memiavl/multitree.go	77.41%	3 Missing and 4 partials ⚠️
sei-db/wal/utils.go	0.00%	4 Missing ⚠️
sei-db/db_engine/pebbledb/mvcc/db.go	57.14%	2 Missing and 1 partial ⚠️
sei-db/db_engine/rocksdb/mvcc/db.go	57.14%	2 Missing and 1 partial ⚠️
sei-db/state_db/sc/store.go	92.85%	2 Missing ⚠️
...-db/tools/cmd/seidb/operations/replay_changelog.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2671      +/-   ##
==========================================
+ Coverage   43.68%   43.76%   +0.08%     
==========================================
  Files        1913     1913              
  Lines      159450   159492      +42     
==========================================
+ Hits        69650    69798     +148     
+ Misses      83393    83277     -116     
- Partials     6407     6417      +10

Flag	Coverage Δ
sei-chain	`45.78% <65.67%> (+0.17%)`	⬆️
sei-cosmos	`38.21% <ø> (-0.01%)`	⬇️
sei-db	`68.72% <57.14%> (-0.71%)`	⬇️
sei-tendermint	`47.25% <ø> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
sei-db/state_db/sc/memiavl/opts.go	`100.00% <ø> (ø)`
sei-db/state_db/ss/store.go	`45.28% <100.00%> (ø)`
...-db/tools/cmd/seidb/operations/replay_changelog.go	`0.00% <0.00%> (ø)`
sei-db/state_db/sc/store.go	`91.30% <92.85%> (+91.30%)`	⬆️
sei-db/db_engine/pebbledb/mvcc/db.go	`55.25% <57.14%> (-0.68%)`	⬇️
sei-db/db_engine/rocksdb/mvcc/db.go	`57.78% <57.14%> (-1.49%)`	⬇️
sei-db/wal/utils.go	`56.71% <0.00%> (ø)`
sei-db/state_db/sc/memiavl/multitree.go	`78.54% <77.41%> (-1.01%)`	⬇️
sei-db/wal/changelog.go	`0.00% <0.00%> (ø)`
sei-db/state_db/sc/memiavl/db.go	`64.53% <62.22%> (+1.39%)`	⬆️
... and 1 more

... and 25 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sei-db/state_db/sc/memiavl/db.go

sei-db/wal/generic_wal/wal.go

sei-db/wal/wal.go

masih

Thanks for this Yiming! Before a full review can I ask you to fix all the ignored errors across the PR please?

I also recommend avoiding the unnecessary package nesting, or the word "generic" in the package name. A typical package structure in Go is typically flatter and less hierarchical than languages like Java. I recommend avoiding common package naming like types because there are already many such packages across the codebase and working with the package name conflicts would require alias, which can lead to inconsistent alias naming and ultimately higher cognitive cycles when reading the code.

sei-db/db_engine/rocksdb/mvcc/db.go

* main: feat: add generic KV interfaces + Pebble adapter (#2666) Make SSTORE chain param height-aware (#2667) fix: cosmos: protect coin denom regexp with a lock (#2660) Install CA certs on Ubuntu base image (#2658) Check storage is non-nil before attempting to close it (#2659)

yzang2019 · 2026-01-08T21:42:10Z

Thanks for this Yiming! Before a full review can I ask you to fix all the ignored errors across the PR please?

I also recommend avoiding the unnecessary package nesting, or the word "generic" in the package name. A typical package structure in Go is typically flatter and less hierarchical than languages like Java. I recommend avoiding common package naming like types because there are already many such packages across the codebase and working with the package name conflicts would require alias, which can lead to inconsistent alias naming and ultimately higher cognitive cycles when reading the code.

Great suggestion! Will look into the naming here

sei-db/state_db/sc/memiavl/multitree.go

+		t.UpdateCommitInfo()
+		replayElapsed := time.Since(startTime).Seconds()
+		t.logger.Info(fmt.Sprintf("Total replayed %d entries in %.1fs (%.1f entries/sec).\n",
+			replayCount, replayElapsed, float64(replayCount)/replayElapsed))


sei-db/wal/wal.go

sei-db/state_db/sc/memiavl/db.go

-			return
+		if wal := db.GetWAL(); wal != nil {
+			catchupStart := time.Now()
+			if err := mtree.Catchup(ctx, wal, db.walIndexDelta, 0); err != nil {


sei-db/wal/wal.go

* main: Fix integration tests to run on release branch and clean up rules (#2696)

sei-db/wal/wal.go

masih

Main blockers:

Potential resource leakage.
Concurrent use safety of WAL

Other than those I left a bunch of feedback across the code

Thanks for working on this @yzang2019 ! 🍻 This is nearly there

.github/workflows/integration-test.yml

sei-db/db_engine/pebbledb/mvcc/db.go

sei-db/db_engine/rocksdb/mvcc/db.go

sei-db/state_db/sc/memiavl/db.go

sei-db/wal/wal.go

masih · 2026-01-14T11:00:03Z

sei-db/wal/wal_test.go

+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			os.WriteFile(filepath.Join(dir, "00000000000000000001"), tc.logs, 0o600)


Assert no error?

masih · 2026-01-14T11:00:49Z

sei-db/wal/wal_test.go

+
+func TestTruncateAfter(t *testing.T) {
+	changelog := prepareTestData(t)
+	defer changelog.Close()


Assert no error, i.e. graceful closure? Ditto for the rest of the tests.

masih · 2026-01-14T11:03:04Z

sei-db/wal/wal_test.go

+}
+
+func TestLogPath(t *testing.T) {
+	path := LogPath("/some/dir")


The "util" LogPath is only used once. It adds an additional 3 lines to the codebase and makes the reader jump into another file to follow what the code does. This is another example of over-refactoring that hardly adds any benefit at all I'm afraid.

sei-db/wal/wal.go

+	go func() {
+		defer walLog.wg.Done()
+		for entry := range ch {
+			bz, err := walLog.marshal(entry)
+			if err != nil {
+				walLog.recordAsyncWriteErr(err)
+				return
+			}
+			nextOffset, err := walLog.NextOffset()
+			if err != nil {
+				walLog.recordAsyncWriteErr(err)
+				return
+			}
+			err = walLog.log.Write(nextOffset, bz)
+			if err != nil {
+				walLog.recordAsyncWriteErr(err)
+				return
+			}
+
+		}
+	}()


sei-db/wal/wal.go

+	go func() {
+		defer walLog.wg.Done()
+		ticker := time.NewTicker(pruneInterval)
+		defer ticker.Stop()
+		for {
+			select {
+			case <-walLog.closeCh:
+				return
+			case <-ticker.C:
+				lastIndex, err := walLog.log.LastIndex()
+				if err != nil {
+					walLog.logger.Error("failed to get last index for pruning", "err", err)
+					continue
+				}
+				firstIndex, err := walLog.log.FirstIndex()
+				if err != nil {
+					walLog.logger.Error("failed to get first index for pruning", "err", err)
+					continue
+				}
+				if lastIndex > keepRecent && (lastIndex-keepRecent) > firstIndex {
+					prunePos := lastIndex - keepRecent
+					if err := walLog.TruncateBefore(prunePos); err != nil {
+						walLog.logger.Error(fmt.Sprintf("failed to prune changelog till index %d", prunePos), "err", err)
+					}
+				}
+			}
+		}
+	}()


blindchaser · 2026-01-14T22:22:28Z

LGTM. not a blocker, but some thoughts on adding test coverage:

Concurrency testing:

It would be valuable to add a testings that exercises concurrent WAL operations (e.g. async writes while truncation/pruning is running, or Close() racing with in-flight writes) to ensure there are no deadlocks, panics, or log corruption under concurrent access.

Isolation testing:

Since WAL/changelog is now scoped per DB, it would be good to add a multi-DB isolation test:
- open two DBs (separate dirs) in the same process,
- interleave commits on both,
- trigger snapshot rewrite and/or WAL truncation on DB A,
- assert DB B’s WAL offsets, CommittedVersion, and state are unaffected.

yzang2019 · 2026-01-14T22:30:04Z

LGTM. not a blocker, but some thoughts on adding test coverage:

Concurrency testing:

It would be valuable to add a testings that exercises concurrent WAL operations (e.g. async writes while truncation/pruning is running, or Close() racing with in-flight writes) to ensure there are no deadlocks, panics, or log corruption under concurrent access.

Isolation testing:

Since WAL/changelog is now scoped per DB, it would be good to add a multi-DB isolation test:

open two DBs (separate dirs) in the same process,

interleave commits on both,

trigger snapshot rewrite and/or WAL truncation on DB A,

assert DB B’s WAL offsets, CommittedVersion, and state are unaffected.

Make sense for concurrency point, for isolation I dont think we need to test that much, since different WAL in different folder should be fully isolated, so I dont think there's any concern around that

* main: fix: lthash worker loop break; remove unreachable digest.Read fallback (#2698)

masih

No obvious blockers that I can see.

I see you have chosen to ignore the note on the resource leak. Please remember to continuously check for usage of those constructors, because not explicitly cleaning up things would leave the correctness to chance - the correctness can rot by accident.

masih · 2026-01-15T09:30:31Z

sei-db/db_engine/pebbledb/db_test.go

 		t.Fatalf("Open: %v", err)
 	}
-	defer func() { _ = db.Close() }()
+	t.Cleanup(func() { require.NoError(t, db.Close()) })


A small note on t.Cleanup vs defer: t.Cleanup is executed after the test belonging to t ends. Whereas, defer is executed when the function returns.

We do not always want one or the other. For example, in a test helper function we usually want t.Cleanup if it creates resources for the tests that are done after function returns.

In this specific case and the remaining changes here it makes no difference in using one or the other. But I thought I point out the difference so that we are careful not to always use either of those.

masih · 2026-01-15T09:31:48Z

sei-db/state_db/sc/memiavl/db.go

-	streamHandler, err := changelog.NewStream(logger, utils.GetChangelogPath(opts.Dir), changelog.Config{
+	// MemIAVL owns changelog lifecycle: always open the WAL here.
+	// Even in read-only mode we may need WAL replay to reconstruct non-snapshot versions.
+	streamHandler, err := wal.NewChangelogWAL(logger, utils.GetChangelogPath(opts.Dir), wal.Config{


So far probably for the current implementation?

There is one way to make sure resource leaks cannot happen no matter how the constructor is used: On partial instantiation, the constructor should clean up after itself.

masih · 2026-01-15T09:34:52Z

sei-db/state_db/sc/memiavl/multitree.go

 		tree.WaitToCompleteAsyncWrite()
 	}

 	if err != nil {


Sorry, but what async process?

Looking at the code the entire operation is executed sequentially. We just happen to pass "process" as a function. Right?

sei-db/wal/wal.go

sei-db/wal/wal_test.go

yzang2019 · 2026-01-15T18:44:02Z

No obvious blockers that I can see.

I see you have chosen to ignore the note on the resource leak. Please remember to continuously check for usage of those constructors, because not explicitly cleaning up things would leave the correctness to chance - the correctness can rot by accident.

Oh actually I didn't ignore that, but the new change doesn't throw any error i WAL open function because we are not checking lastOffset any more, so it won't have that issue.

Haven't really checked other places such as MemIAVL open function though

Refactor changelog to generic WAL

302783e

yzang2019 requested a review from blindchaser January 7, 2026 06:56

github-advanced-security bot found potential problems Jan 7, 2026

View reviewed changes

sei-db/state_db/sc/memiavl/db.go Fixed Show fixed Hide fixed

sei-db/wal/generic_wal/wal.go Fixed Show fixed Hide fixed

sei-db/wal/wal.go Fixed Show fixed Hide fixed

Add unit test and extract wal initializer

5b81142

yzang2019 requested review from Kbhat1, jewei1997 and masih January 7, 2026 07:36

yzang2019 added the non-app-hash-breaking label Jan 7, 2026

masih reviewed Jan 7, 2026

View reviewed changes

sei-db/db_engine/rocksdb/mvcc/db.go Outdated Show resolved Hide resolved

yzang2019 added 4 commits January 8, 2026 03:03

Fix go lint

08deff1

Fix race condition

55d217c

Address comment

1d5f2bb

Extract the truncation logic out

24749b2

github-advanced-security bot found potential problems Jan 8, 2026

View reviewed changes

yzang2019 added 2 commits January 8, 2026 14:44

Flatten WAL packages

39f8db8

Fix lint

bb0511a

github-advanced-security bot found potential problems Jan 8, 2026

View reviewed changes

sei-db/wal/wal.go Fixed Show fixed Hide fixed

sei-db/wal/wal.go Fixed Show fixed Hide fixed

Fix go lint

456f02b

blindchaser reviewed Jan 9, 2026

View reviewed changes

sei-db/state_db/sc/memiavl/db.go Outdated Show resolved Hide resolved

yzang2019 added 2 commits January 9, 2026 09:04

Rename wal subscriber

17f2752

Catch up from the correct range instead of earliest offset

6a1bc68

github-advanced-security bot found potential problems Jan 9, 2026

View reviewed changes

sei-db/state_db/sc/memiavl/db.go Fixed Show fixed Hide fixed

yzang2019 added 3 commits January 9, 2026 23:05

Fix committed version to return persisted height

668ce4c

Fix commitstore open wal

29a84be

Fix go lint

b1b490b

github-advanced-security bot found potential problems Jan 10, 2026

View reviewed changes

blindchaser reviewed Jan 13, 2026

View reviewed changes

sei-db/wal/wal.go Outdated Show resolved Hide resolved

yzang2019 added 4 commits January 13, 2026 15:11

Add changelog back and upgrade tidywal version

d6005ae

Fix go mod

9f3cf3f

Merge branch 'main' into yzang/redesign-changelog

646d500

* main: Fix integration tests to run on release branch and clean up rules (#2696)

remove checkerror in WAL

f2d1197

yzang2019 requested a review from blindchaser January 14, 2026 01:55

github-advanced-security bot found potential problems Jan 14, 2026

View reviewed changes

sei-db/wal/wal.go Fixed Show fixed Hide fixed

masih reviewed Jan 14, 2026

View reviewed changes

yzang2019 added 6 commits January 14, 2026 08:17

Address comments to make integration test simpler and db not panic

79324e2

Remove unnecessary config overrides and fix resource leak

12d563a

Fix type and add error check for last and first index

6a5dc4a

Fix locking issue and thread safety issue for WAL

136ffa8

Fix tests

65bceb2

Fix close logic

07859a0

github-advanced-security bot found potential problems Jan 14, 2026

View reviewed changes

Require error checking for close

2c3d509

yzang2019 requested a review from masih January 14, 2026 22:26

yzang2019 added 2 commits January 14, 2026 14:49

Add unit test for concurrency and deadlock test

06dee5f

Merge branch 'main' into yzang/redesign-changelog

718e07c

* main: fix: lthash worker loop break; remove unreachable digest.Read fallback (#2698)

blindchaser approved these changes Jan 14, 2026

View reviewed changes

masih approved these changes Jan 15, 2026

View reviewed changes

yzang2019 added 4 commits January 15, 2026 17:01

Simplify wal unit test

08508a2

Fix ignore error

f10fb45

Addree more comments

a387f13

Merge latest

784f7c2

yzang2019 merged commit ef2912e into main Jan 16, 2026
39 checks passed

yzang2019 deleted the yzang/redesign-changelog branch January 16, 2026 06:41

Refactor changelog to generic WAL #2671

Refactor changelog to generic WAL #2671

Uh oh!

Conversation

yzang2019 commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your changes and provide context

Testing performed to validate your change

Uh oh!

github-actions bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

masih left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

yzang2019 commented Jan 8, 2026

Uh oh!

Check notice

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Check warning

Uh oh!

Uh oh!

masih 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

masih Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

masih Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

yzang2019 Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

masih Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Check notice

Check notice

blindchaser commented Jan 14, 2026

Uh oh!

yzang2019 commented Jan 14, 2026

Uh oh!

masih left a comment

Choose a reason for hiding this comment

Uh oh!

masih Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

masih Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

masih Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yzang2019 commented Jan 7, 2026 •

edited

Loading

github-actions bot commented Jan 7, 2026 •

edited

Loading

codecov bot commented Jan 7, 2026 •

edited

Loading

masih left a comment •

edited

Loading

yzang2019 commented Jan 15, 2026 •

edited

Loading