Skip to content

[Backend] Reset datascript tx counter#2771

Merged
nezaj merged 3 commits into
mainfrom
store-fix
Jun 29, 2026
Merged

[Backend] Reset datascript tx counter#2771
nezaj merged 3 commits into
mainfrom
store-fix

Conversation

@nezaj

@nezaj nezaj commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Datascript uses 32-bit integers for tx-ids. Given enough transactions we will exhaust that. This PR makes it so that when we are within ~1M tx-ids from our max we reset the app's tx-id back to the starting point.

Start / max values come from https://github.com/tonsky/datascript/blob/master/src/datascript/db.cljc

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 5ce0bb84-0576-475d-a042-4bfc316facc2

📥 Commits

Reviewing files that changed from the base of the PR and between f8ab765 and 1ad5786.

📒 Files selected for processing (2)
  • server/src/instant/reactive/store.clj
  • server/test/instant/reactive/store_test.clj
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/src/instant/reactive/store.clj

📝 Walkthrough

Walkthrough

Adds overflow prevention for Datascript 32-bit transaction IDs in the reactive store. Introduces reset-conn-tx! to rebuild a connection's DB by re-encoding all datoms at a fixed starting tx-id, a maybe-reset-conn-tx! gate checked at every transact! call, a feature flag conn-tx-reset-disabled?, and tests covering reset behavior.

Changes

Datascript TX-ID Reset

Layer / File(s) Summary
Feature flag: conn-tx-reset-disabled?
server/src/instant/flags.clj
Adds conn-tx-reset-disabled? accessor that reads the :conn-tx-reset-disabled? toggle via toggled?.
reset-conn-tx! and maybe-reset-conn-tx! wired into transact!
server/src/instant/reactive/store.clj
Defines ds-tx0 and ds-tx-reset-threshold constants; implements reset-conn-tx! to rewrite datoms under the connection lock; adds maybe-reset-conn-tx! gate that checks the threshold and flag, called at the start of every transact!.
Unit test for reset-conn-tx!
server/test/instant/reactive/store_test.clj
Advances a connection's tx counter, calls reset-conn-tx!, and asserts :max-tx equals ds-tx0, datoms are preserved, :max-eid is unchanged, and lookup entities resolve correctly.
Preserve max-eid across reset
server/test/instant/reactive/store_test.clj
Covers a retract-and-reset case where reset-conn-tx! keeps the prior :max-eid and later transactions allocate a new eid above the retracted maximum.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: resetting Datascript transaction counters.
Description check ✅ Passed The description is directly related and explains the tx-id exhaustion fix and reset threshold.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch store-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
server/src/instant/reactive/store.clj (1)

462-464: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Re-check the threshold while holding the conn lock.

Concurrent callers can all pass the pre-lock threshold check, then serialize multiple full DB rebuilds even after the first reset lowered :max-tx. Re-check inside the same lock before rebuilding to avoid a reset storm on a large store.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/src/instant/reactive/store.clj` around lines 462 - 464, Re-check the
tx reset threshold inside the same lock before rebuilding, because
`reset-conn-tx!` can be entered by multiple concurrent callers after they all
pass the outer `:max-tx` check. Update the `reset-conn-tx!` flow in `store.clj`
so the threshold comparison against `ds-tx-reset-threshold` is performed again
while holding the conn lock, and only trigger the full DB rebuild if `:max-tx`
is still above the threshold and `flags/conn-tx-reset-disabled?` is false.
server/test/instant/reactive/store_test.clj (1)

490-519: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for the threshold gate and disable flag.

This test covers direct reset, but the production path is maybe-reset-conn-tx! plus conn-tx-reset-disabled?. Add focused assertions for “resets over threshold” and “does not reset when disabled” so the safety switch is covered.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@server/test/instant/reactive/store_test.clj` around lines 490 - 519, The
current test only covers rs/reset-conn-tx!, but it does not exercise the
production gate in maybe-reset-conn-tx! or the disable flag check in
conn-tx-reset-disabled?. Add focused test coverage that verifies a connection is
reset when the tx count exceeds the threshold, and that no reset happens when
the disable flag is enabled, using rs/maybe-reset-conn-tx! and
rs/conn-tx-reset-disabled? so both safety paths are covered.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@server/src/instant/reactive/store.clj`:
- Around line 452-455: Preserve the conn’s :max-eid when rebuilding the
Datascript store from d/datoms in the store reset path, since init-db recomputes
it from live datoms and can reuse a retracted highest eid. Update the rebuild
logic around the conn reset to carry forward the previous max-eid value, and
verify the behavior in a regression test that exercises a fully retracted
highest eid so the next transaction does not collide with stale cache keys.

---

Nitpick comments:
In `@server/src/instant/reactive/store.clj`:
- Around line 462-464: Re-check the tx reset threshold inside the same lock
before rebuilding, because `reset-conn-tx!` can be entered by multiple
concurrent callers after they all pass the outer `:max-tx` check. Update the
`reset-conn-tx!` flow in `store.clj` so the threshold comparison against
`ds-tx-reset-threshold` is performed again while holding the conn lock, and only
trigger the full DB rebuild if `:max-tx` is still above the threshold and
`flags/conn-tx-reset-disabled?` is false.

In `@server/test/instant/reactive/store_test.clj`:
- Around line 490-519: The current test only covers rs/reset-conn-tx!, but it
does not exercise the production gate in maybe-reset-conn-tx! or the disable
flag check in conn-tx-reset-disabled?. Add focused test coverage that verifies a
connection is reset when the tx count exceeds the threshold, and that no reset
happens when the disable flag is enabled, using rs/maybe-reset-conn-tx! and
rs/conn-tx-reset-disabled? so both safety paths are covered.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 34bdefe2-f64e-4935-85d4-0b230bfa2dc0

📥 Commits

Reviewing files that changed from the base of the PR and between e464d8f and f8ab765.

📒 Files selected for processing (3)
  • server/src/instant/flags.clj
  • server/src/instant/reactive/store.clj
  • server/test/instant/reactive/store_test.clj

Comment thread server/src/instant/reactive/store.clj Outdated

@dwwoelfel dwwoelfel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool!

@nezaj nezaj merged commit 8f8655d into main Jun 29, 2026
34 checks passed
@nezaj nezaj deleted the store-fix branch June 29, 2026 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants