Use the HC-Tree journal on flagged leaders and followers by tylerkaraszewski · Pull Request #2818 · Expensify/Bedrock

tylerkaraszewski · 2026-09-23T19:42:44Z

Reviewer Notes

There are a ton of test changes in this PR but the bulk of the actual change is approximately 400 lines of changes in SQLite.cpp.

The rollout plan for this:

Enable it on a single webrock follower with the command line flag, and wait and see how it works.
After that, we can add a second webrock follower, and allow it to acquire a fairly long journal of hct_journal commits.
Make the second follower leader and make sure that we can both lead and follow with the new journal.
Enable it on db2.rno to test on auth.

After that, we can move forward with the next HC-Tree changes.

Explanation of Change

This adds an -hctreeExperimentalMode flag to bedrock. This flag is intended to be re-used for future changes as what counts as "experimental" changes over time. For this change, this allows us to start using HC-Tree's built-in journal.

-hctreeExperimentalMode is ignored on databases that are not HC-Tree, i.e., WAL2 dbs.

The rest of this describes what -hctreeExperimentalMode does in this change, when it is enabled on a HC-Tree database.

Followers, or essentially any node that is not leader (i.e., this includes nodes that are SYNCHRONIZING, etc) write replicated commits to hct_journal instead of the legacy journals. When leader is not also on -hctreeExperimentalMode (more on if leader is in -hctreeExperimentalMode below), then these look largely like existing journal commits - namely, commit conflicts do not allocate blank journal entries on the leader. The layout is different - hct_journal does not have a separate hash column, and so we concatenate $HASH:$QUERY in hct_journal.

In a transitional phase, it is possible that nodes will need to synchronize commits that are in both the old and new journals. This is handled by looking up commits to send to SYNCHRONIZING peers in both journal types. They are sent in the existing format, and can be committed to the old or new journals as appropriate on the receiver (only other HC-Tree nodes with -hctreeExperimentalMode will write them to hct_journal).

One-way switch

Once a DB has had -hctreeExperimentalMode turned on and hct_journal initialized, we do not allow switching back. Bedrock will log an error and exit if you start this DB without -hctreeExperimentalMode after this point. The expected way to disable this on a node is to restore the node from a backup with no hct_journal.

This prevents interleaved journal entries in different journal types that break what's required for contiguous journal entries on leader with -hctreeExperimentalMode enabled.

Enabling -hctreeExperimentalMode on leader.

When leader has -hctreeExperimentalMode it can result in blank journal entries in the case of conflicts. These are recorded and broadcast to followers, in accordance with the HC-Tree spec. This is expected but this will be the first time this is tested in production.

BlockWrites
We have a BlockWrites command that does what it says. The behavior of this changes - previously, it froze transactions in place, refusing to let them start new writes. This provided ambiguous behavior (and possible race conditions) if you called this on a transaction that was mid-write. The behavior for blocking writes now allows all existing write transactions to complete.

Minor differences

On the first startup with the new flag, the newest row in the existing journal is moved to hct_journal to seed it.
New databases now how their first real entry at ID 2, with a blank entry at ID 1, even for WAL2. This does not matter in production.
The deleter thread also trims hct_journal

Fixed Issues

For https://github.com/Expensify/Expensify/issues/337537

Tests

Several new tests added.

chatgpt-codex-connector · 2026-09-29T15:45:21Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-29T15:51:06.631057Z	`143da21`	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

danieldoglas · 2026-09-30T22:19:53Z

+        SQResult latest;
+        SASSERT(!SQuery(db, "SELECT MAX(id) FROM (" + _getJournalQuery(journalNames, {"SELECT MAX(id) AS id FROM"}, true) + ")", latest));
+        if (latest.empty() || latest[0][0].empty()) {
+            SASSERT(!SQuery(db, "INSERT INTO " + journalNames.front() + " (id, query, hash) VALUES (1, X'', '')"));


what does X'' do here?

This is binary data and inserts a blank BLOB instead of a blanks STRING. It doesn't make a huge practical difference here when it's blank, but since we're inserting compressed binary data in the journal, this just matches that.

danieldoglas · 2026-09-30T22:24:36Z

+    }
+
+    if (experimentalHCTree && !hctJournalExists) {
+        initializeHCTreeJournal(db, journalNames);


This is sending the same db handle used in the insert above, but we don't commit that insert. In this function, we call begin/create table/commit|rollback. Is that a problem? Or since we didn't create a BEGIN/COMMIT for the insert above, SQlite is doing it automatically?

It does. I will add a comment about it next to the insert.

danieldoglas · 2026-09-30T22:26:15Z

+            {":cid", Parameter::i(legacyCommitID)},
+            {":query", Parameter::blob(legacy[0]["hash"] + ":" + legacy[0]["query"])},


I like this. Should we start using this format in Auth queries?

I added this here in May:
#2600

And also added it to Auth:
https://github.com/Expensify/Auth/pull/21810

So there's no reason not to use it.

danieldoglas · 2026-09-30T22:30:14Z

+    if (!_journalNames.empty() && (!experimentalHCTree || (_sharedData.legacyMaxID && fromIndex <= _sharedData.legacyMaxID))) {
+        const string where = " WHERE id >= " + SQ(fromIndex) + (toIndex ? " AND id <= " + SQ(toIndex) : "");
+        for (const string& name : _journalNames) {
+            sources.push_back("SELECT id, query, hash FROM " + name + where);
+        }
+    }
+    if (experimentalHCTree && (!toIndex || toIndex >= _sharedData.hctMinID)) {
+        sources.push_back(_getHCTreeJournalQuery() + " WHERE cid >= " + SQ(fromIndex) +
+                          (toIndex ? " AND cid <= " + SQ(toIndex) : ""));
+    }


A comment here would be helpful

danieldoglas · 2026-09-30T22:42:32Z

        SINFO("writeLock Locked");
+        uint64_t count;
+        while ((count = _sharedData.openWriteTransactionCount.load())) {
+            _sharedData.openWriteTransactionCount.wait(count);


TIL that atomic has a wait implementation

It's relatively new in C++20.

danieldoglas · 2026-09-30T22:43:00Z

+    // After locking writeLock, we wait for all existing writers to finish. They will likely require commitLock
+    // as well, which we only acquire once they all complete. No new writes can start once we lock writeLock.
+    // This function blocks until in-progress writes finish.
    try {


This is a pretty nice change that make things easier to understand. Thanks

danieldoglas · 2026-09-30T22:57:37Z

+        rollback();
+        STHROW("Attempted to begin transaction while in invalid state: _uncommittedQuery not empty");
+    }
+    if (type == TRANSACTION_TYPE::EXCLUSIVE) {


Why only on type exclusive?

and if this is readlly necessary, why are we doing it here instead of in like 745 for example?

So in general, a transaction is marked as a write transaction on the first attempt it makes to write. This is used for when we need to let all of the write transactions complete - anything that was marked as write gets to complete, but nothing not marked as write gets to start writing. If any non-write transaction wants to write, it will get blocked attempting to acquire writeLock.

We simplified the rules around acquiring writeLock and commitLock so that we always lock writeLock first, when we start writing, and then commitLock later, when we want to commit.

EXCLUSIVE transactions lock commitLock immediately, which is how they enforce exclusiveness, so in order to avoid deadlocks, they also need to lock writeLock immediately - before they lock commitLock, so we need to do this right away here.

But yes, it can move to line 746, I'll move it.

danieldoglas · 2026-09-30T23:11:39Z

+        if (SToUInt64(bounds[0][0]) >= oldestCommitToKeep || bounds[0][0] == bounds[0][1]) {
+            return true;
+        }
+        // Delete a prefix, never the final entry. HC-Tree requires the journal


I don't understand what this means. What's prefix/final entry?

danieldoglas · 2026-09-30T23:11:47Z

+        uint64_t oldestCommitToKeep = commitCount < _maxJournalSize ? 0 : commitCount - _maxJournalSize;
+        if (state.hashCommitID) {
+            oldestCommitToKeep = min(oldestCommitToKeep, state.hashCommitID);
+        }
+        if (oldestCommitToKeep <= _sharedData.hctMinID) {
+            return true;
+        }
+        SQResult bounds;
+        if (!read("SELECT MIN(cid), MAX(cid) FROM hct_journal", bounds) || bounds.empty() || bounds[0][0].empty()) {
+            return false;
+        }
+        if (SToUInt64(bounds[0][0]) >= oldestCommitToKeep || bounds[0][0] == bounds[0][1]) {
+            return true;
+        }


Coments would help here

…-follower-journal-commits" This reverts commit 740e769, reversing changes made to 6df552e. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

tylerkaraszewski · 2026-10-05T23:59:56Z

The Auth-side test compatibility fixes for the re-land are in Auth PR 25180: local test handles now match the servers' native-journal mode, and the query-plan test uses its own table instead of the legacy journal. The affected fixtures pass against both Bedrock versions. The full native Auth suite still has 30 failures, including a reproduced read-corruption error and shutdown hang; details are in the Auth PR.

Revert the revert in 11fbdb2, restoring the original change from Expensify#2818 before applying follow-up fixes.

tylerkaraszewski added 3 commits September 23, 2026 12:41

Add HC-Tree follower journal storage and mixed-journal reads

5f06553

Gate HC-Tree follower journal mode on a node flag

a48089c

Document the HC-Tree follower journal rollout

5ab08c5

tylerkaraszewski self-assigned this Sep 23, 2026

tylerkaraszewski added 9 commits September 23, 2026 12:45

Use a general HC-Tree experimental mode flag

d2ec65e

Use the official HC-Tree SQLite header

41e28d2

Set HC-Tree experimental mode on SQLite at startup

b1cf6d2

Enable experimental HC-Tree mode in HC-Tree test servers

b808c99

manual cleanup

3acb042

Use HC-Tree leader commits and drain active writers on mode changes

943ad60

Test HC-Tree leader failover and write transaction draining

c3806c9

Account for the initial blank commit in journal deleter tests

9237754

Document experimental HC-Tree leader commits

473dedd

tylerkaraszewski changed the title ~~Use the HC-Tree journal on flagged followers~~ Use the HC-Tree journal on flagged leaders and followers Sep 23, 2026

tylerkaraszewski added 16 commits September 23, 2026 16:12

Report HC-Tree CID allocation from SQLite commits

cf4671e

Exclude upstream HC-Tree header from style checks

2a55796

Check both journals when waiting for conflict spam replication

076728b

Corrupt the HC-Tree hash in fork detection tests

c8f910f

Skip legacy journal creation in experimental HC-Tree mode

7f321ce

Require adjacent legacy and HC-Tree journals before leadership

f83e20e

Test HC-Tree journal migration and promotion boundaries

eb6bda5

Document contiguous journal boundaries for HC-Tree leadership

29a5021

Generate deterministic blank HC-Tree commits in the test plugin

5670153

Verify blank commits replicate live and synchronize across batches

61a1ee3

Make HC-Tree journal initialization a one-way transition

a4c82dc

Inspect stopped fork-test databases without Bedrock initialization

1eb0f4d

Test one-way HC-Tree initialization and rejected startup preservation

172f05b

Document the permanent HC-Tree journal transition

388bb12

Route journal reads using immutable startup bounds

b8cda52

Stop trimming drained legacy journals after HC-Tree cutover

6878dbe

tylerkaraszewski added 14 commits September 25, 2026 17:28

Remove unused conflict-spam queries and bound its catch-up loop

ff685ca

Make failover response validation effective and simplify client cleanup

ce92ccb

Clarify HC-Tree initialization and retention documentation

07431b0

Note beginOnly removal for parallel replication

1ccad43

Merge latest HC-Tree fix

6eec0d2

Remove the HC-Tree journal anchor workaround

2fb5dd0

Revert broken style script

df17953

Update some comments to be more human-friendly

58e5034

Manual review updates

3598cc8

Clarify journal trimming method comments

0f952b0

Clearer comment

7430839

overzealous AI test

ca15f34

Remove trailing whitespace in journal trimming comment

7c0b3c7

fix broken test

143da21

tylerkaraszewski requested a review from danieldoglas September 28, 2026 21:00

tylerkaraszewski marked this pull request as ready for review September 29, 2026 15:45

danieldoglas reviewed Sep 30, 2026

View reviewed changes

tylerkaraszewski added 3 commits October 1, 2026 09:30

review feedback

621a0ac

Merge branch 'main' into tyler-hctree-follower-journal-commits

6b8f377

One more comment change

60bc911

tylerkaraszewski requested a review from danieldoglas October 1, 2026 16:37

danieldoglas approved these changes Oct 2, 2026

View reviewed changes

tylerkaraszewski merged commit 740e769 into main Oct 2, 2026
9 of 10 checks passed

tylerkaraszewski deleted the tyler-hctree-follower-journal-commits branch October 2, 2026 16:47

robertjchen mentioned this pull request Oct 2, 2026

Revert "Use the HC-Tree journal on flagged leaders and followers" #2833

Merged

tylerkaraszewski mentioned this pull request Oct 5, 2026

Restore HC-Tree journal mode with read-only commit handling #2836

Merged

heekinho pushed a commit to heekinho/Bedrock that referenced this pull request Oct 8, 2026

Restore HC-Tree journal commits on flagged nodes

c61278b

Revert the revert in 11fbdb2, restoring the original change from Expensify#2818 before applying follow-up fixes.

		{":cid", Parameter::i(legacyCommitID)},
		{":query", Parameter::blob(legacy[0]["hash"] + ":" + legacy[0]["query"])},

Conversation

tylerkaraszewski commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Notes

Explanation of Change

Fixed Issues

Tests

Uh oh!

chatgpt-codex-connector Bot commented Sep 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

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!

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!

Uh oh!

tylerkaraszewski commented Oct 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tylerkaraszewski commented Sep 23, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Sep 29, 2026 •

edited

Loading