Skip to content

perf(tm2/bptree): find the oldest and newest version with two seeks#5979

Open
davd-gzl wants to merge 8 commits into
gnolang:masterfrom
davd-gzl:fix/bptree-bounded-version-discovery
Open

perf(tm2/bptree): find the oldest and newest version with two seeks#5979
davd-gzl wants to merge 8 commits into
gnolang:masterfrom
davd-gzl:fix/bptree-bounded-version-discovery

Conversation

@davd-gzl

@davd-gzl davd-gzl commented Jul 18, 2026

Copy link
Copy Markdown
Member

Follow-up to #5937.

Opening the store reads every stored version to find just the oldest and the newest. gno.land keeps 705,600 of them, and every query at a height pays for it.

Version keys are already sorted: R followed by the version as 8 bytes, biggest part first.

R…0003   ← oldest
R…0004
R…0005
  ⋮
R…AC44   ← newest

Before, per query:

read R…0003, R…0004, R…0005 … R…AC44     705,600 reads
track the smallest and largest seen

After:

seek to the first R key    → oldest
seek to the last R key     → newest      2 reads

The values were always at the ends; the scan just never looked there. A new test covers the case where pruning removes the oldest versions, so the first key is no longer version 1.

Opening the store this way can also write to the database, since loading rebuilds a stale lookup index. Left for a follow-up, written up in the ADR: skipping the rebuild alone is unsafe, because a read trusts an index entry on its version alone and a stale index would return wrong values.

discoverVersions scanned every PrefixRoot key to take the min and max
version. MutableTree.Load calls it, and the rootmulti immutable store opens
a tree per ABCI query at a height, so every custom query reran the full
scan. The cost is linear in the retained-version count, and the gno.land
default prune strategy keeps 705,600.

Root keys are PrefixRoot-version-BE, so key order is version order: the
first forward key is the smallest version and the first reverse key is the
largest. Seek both ends instead. rootDBKey is the only writer under
PrefixRoot and always emits 9 bytes, so the result matches the old scan for
every reachable input.
@Gno2D2

Gno2D2 commented Jul 18, 2026

Copy link
Copy Markdown
Collaborator

🛠 PR Checks Summary

🔴 Pending initial approval by a review team member, or review from tech-staff

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):

🟢 Maintainers must be able to edit this pull request (more info)
🔴 Pending initial approval by a review team member, or review from tech-staff

☑️ 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:
Debug
Automated Checks
Maintainers must be able to edit this pull request (more info)

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 The pull request was created from a fork (head branch repo: davd-gzl/gno)

Then

🟢 Requirement satisfied
└── 🟢 Maintainer can modify this pull request

Pending initial approval by a review team member, or review from tech-staff

If

🟢 Condition met
└── 🟢 And
    ├── 🟢 The base branch matches this pattern: ^master$
    └── 🟢 Not (🔴 Pull request author is a member of the team: tech-staff)

Then

🔴 Requirement not satisfied
└── 🔴 If
    ├── 🔴 Condition
    │   └── 🔴 Or
    │       ├── 🔴 At least one of these user(s) reviewed the pull request: [aronpark1007 davd-gzl jefft0 notJoon omarsy MikaelVallenet] (with state "APPROVED")
    │       ├── 🔴 At least 1 user(s) of the team tech-staff reviewed pull request
    │       └── 🔴 This pull request is a draft
    └── 🔴 Else
        └── 🔴 And
            ├── 🟢 This label is applied to pull request: review/triage-pending
            └── 🔴 On no pull request

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

@davd-gzl davd-gzl changed the title perf(tm2/bptree): bound version discovery to two seeks perf(tm2/bptree): find the oldest and newest version with two seeks Jul 21, 2026
davd-gzl and others added 4 commits July 21, 2026 15:36
The comment justified leaving these snapshots unprotected by saying prune
and queries are serialized by the ABCI mutex. They are not: the query
connection has its own mutex in proxy/client.go, separate from the one
consensus and mempool share, so the two genuinely run concurrently.

The code is safe for different reasons, and those are the ones a future
maintainer needs when deciding whether a new snapshot path can skip reader
registration: a query never touches the consensus tree, because rootmulti
builds a separate store with its own reader map, and the snapshot
registers no reader so it cannot block prune.
Replaces the prose describing the key layout with the layout itself and
the value each direction returns.
@davd-gzl
davd-gzl marked this pull request as ready for review July 21, 2026 14:04
@Gno2D2 Gno2D2 added the review/triage-pending PRs opened by external contributors that are waiting for the 1st review label Jul 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

📦 🌐 tendermint v2 Issues or PRs tm2 related review/triage-pending PRs opened by external contributors that are waiting for the 1st review

Projects

Development

Successfully merging this pull request may close these issues.

2 participants