Skip to content

refactor(dbkit/sqlite): wait for a lock through modernc.org/sqlite v1.61.0's busy handler - #108

Merged
SirLouen merged 3 commits into
mainfrom
refactor/107-driver-busy-handler
Oct 22, 2026
Merged

SirLouen merged 3 commits into
mainfrom
refactor/107-driver-busy-handler

Conversation

@SirLouen

@SirLouen SirLouen commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Closes #107

What

  • Breaking. dbkit/sqlite now needs modernc.org/sqlite v1.61.0 and modernc.org/libc v1.79.0. LibcVersion is v1.79.0.
  • dbkit's own busy handler is gone, and so is its read of a private field of the driver.
  • Each connection now waits for a lock through the driver's own busy handler. That wait ends with the call's context.
  • A new connection sets its busy timeout back to BusyTimeout after PRAGMA optimize, as 0.1.0 did. PRAGMA busy_timeout reads BusyTimeout again.
  • sql.Conn.Raw hands out the driver's own connection again.
  • Breaking. A BeginTx error after its context ends now matches only the context error. It still matches dbkit.ErrBusy when opening its connection answered busy.
  • Breaking. A lock wait can start while reading a later row of a query, or while closing its rows. That wait now ends at BusyTimeout, not with the query's context.
  • Breaking. A waiting connection tries the lock again on SQLite's own schedule, up to every 100ms. BusyPoll now only paces opening a connection.

Why

modernc.org/sqlite v1.61.0 now ends a busy wait with the call's context by itself. dbkit no longer needs its own handler, or the private field read that any driver release could break. dbkit/sqlite 0.2.0 already misbehaves on v1.61.0: sqlitetest.CheckLibc fails, and the BeginTx error changes from run to run. So a downstream application that pulls v1.61.0 through another dependency breaks today.

Testing Instructions

Run the testing toolchain in dbkit/sqlite.

Summary by CodeRabbit

  • Bug Fixes

    • SQLite connections now consistently honor configured busy-timeout settings when waiting on locks.
    • Context cancellation and lock waits now return more predictable errors, including during later query steps and row closing.
    • Busy-timeout restoration is more reliable after connection optimization.
    • sql.Conn.Raw now provides access to the underlying driver connection as expected.
  • Documentation

    • Updated SQLite dependency requirements and documented revised timeout, transaction, and connection behavior.
    • Clarified busy-polling option behavior.

@SirLouen SirLouen self-assigned this Oct 22, 2026
@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: a3b4da33-6dbe-4b8e-9e30-5107b1d5551a
📥 Commits

Reviewing files that changed from the base of the PR and between bcacef0 and 5f4155f.

📒 Files selected for processing: 1
  • dbkit/sqlite/busy_test.go

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: f8548c45-4f08-46e8-a22d-a31f49a5d9c6
📥 Commits

Reviewing files that changed from the base of the PR and between 0cbf283 and bcacef0.

⛔ Files ignored due to path filters: 1
  • dbkit/sqlite/go.sum is excluded by !**/*.sum
📒 Files selected for processing: 16
  • dbkit/sqlite/.golangci.yml
  • dbkit/sqlite/CHANGELOG.md
  • dbkit/sqlite/busy.go
  • dbkit/sqlite/busy_calls_internal_test.go
  • dbkit/sqlite/busy_internal_test.go
  • dbkit/sqlite/busy_test.go
  • dbkit/sqlite/classify.go
  • dbkit/sqlite/connector.go
  • dbkit/sqlite/connector_internal_test.go
  • dbkit/sqlite/connector_test.go
  • dbkit/sqlite/go.mod
  • dbkit/sqlite/options.go
  • dbkit/sqlite/rules_test.go
  • dbkit/sqlite/share_test.go
  • dbkit/sqlite/sqlitetest/faults_test.go
  • dbkit/sqlite/sqlitetest/template_test.go
💤 Files with no reviewable changes: 4
  • dbkit/sqlite/.golangci.yml
  • dbkit/sqlite/busy.go
  • dbkit/sqlite/busy_calls_internal_test.go
  • dbkit/sqlite/busy_internal_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The SQLite module updates its driver dependencies and removes dbkit’s custom busy handler. Connection setup restores the configured SQLite busy timeout after optimization. Tests now cover driver-managed busy waits and configured timeout behavior.

Changes: SQLite busy handling migration

Layer / File(s) Summary
Update the driver and remove the custom handler
dbkit/sqlite/go.mod, dbkit/sqlite/classify.go, dbkit/sqlite/.golangci.yml, dbkit/sqlite/busy.go, dbkit/sqlite/busy_calls_internal_test.go, dbkit/sqlite/busy_internal_test.go, dbkit/sqlite/CHANGELOG.md
The module updates modernc.org/sqlite and modernc.org/libc, and updates LibcVersion. It removes the custom busy handler, private driver-field access, and associated internal tests.
Restore busy timeout during connection setup
dbkit/sqlite/connector.go, dbkit/sqlite/connector_internal_test.go, dbkit/sqlite/connector_test.go, dbkit/sqlite/options.go
The connector restores the configured busy timeout after optimization, using a context that cannot be canceled. Tests cover restoration errors, context cancellation, and access to the underlying driver connection through sql.Conn.Raw.
Validate busy waits and configured timeouts
dbkit/sqlite/busy_test.go, dbkit/sqlite/rules_test.go, dbkit/sqlite/share_test.go, dbkit/sqlite/sqlitetest/faults_test.go, dbkit/sqlite/sqlitetest/template_test.go
Busy-operation tests check context errors and timeout behavior. Connection tests now expect SQLite’s busy timeout to match the configured value.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant connector
  participant SQLiteConnection
  participant moderncSQLite
  Caller->>connector: Connect
  connector->>SQLiteConnection: Lower busy timeout and optimize
  connector->>SQLiteConnection: Restore configured busy timeout
  connector-->>Caller: Return connection
  Caller->>moderncSQLite: Send SQL operation with context
  moderncSQLite-->>Caller: Return result or context error
Loading
Merge Risk: ⚪ Minimal · up to bcace

No confirmed issue blocks the SQLite driver migration after normal checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 10 files. (2 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main change: dbkit/sqlite now relies on modernc.org/sqlite v1.61.0 for busy-handler lock waits.
Linked Issues check Passed Issue #107 is directly linked and remains open. The PR updates modernc.org/sqlite to v1.61.0, updates modernc.org/libc and LibcVersion to v1.79.0, removes the custom busy handler and private dri…
Out of Scope Changes check Passed The changes stay within Issue #107. Production changes implement driver-owned busy handling, timeout restoration, dependency updates, and BusyPoll behavior. The test, changelog, lint configuration, …
Full details: Linked Issues check

Explanation

Issue #107 is directly linked and remains open. The PR updates modernc.org/sqlite to v1.61.0, updates modernc.org/libc and LibcVersion to v1.79.0, removes the custom busy handler and private driver-field access, restores BusyTimeout after PRAGMA optimize, and preserves the timeout through PRAGMA busy_timeout. The updated tests cover context behavior, later row operations, row closing, driver connections from sql.Conn.Raw, and BusyPoll opening behavior.

Full details: Out of Scope Changes check

Explanation

The changes stay within Issue #107. Production changes implement driver-owned busy handling, timeout restoration, dependency updates, and BusyPoll behavior. The test, changelog, lint configuration, and option-comment changes support or document those objectives. No unrelated change is demonstrated.

Full details: Docstring Coverage

Explanation

Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 38 functions across 10 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

codescene-access[bot]

This comment was marked as outdated.

@greptile-apps

greptile-apps Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] No outstanding findings block merging the reviewed changes.

Summary

This PR replaces dbkit's SQLite busy handler with the driver's own handler.

  • SQLite's driver now owns lock waits for each connection.
  • New SQLite connections restore the configured lock timeout after optimization.
  • The SQLite package now requires newer modernc driver and libc versions.

No new actionable issues were found.

Previous concern: SirLouen disputed the claim that Rows.Err() must return the context error, explaining that a failed driver Next marks the rows finished and preserves the driver's busy error. The original finding was incorrect. The current test retains the busy-error assertion and starts its cancellation timer immediately before the second step.

Reviews (2) · Last reviewed commit: "test(dbkit/sqlite): start the later-step..." · Reviewed by Greptile

Comment thread dbkit/sqlite/busy_test.go

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Passed
4 Quality Gates Passed

See analysis details in CodeScene

Quality Gate Profile: Clean Code Collective
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@SirLouen
SirLouen merged commit fbeed38 into main Oct 22, 2026
42 checks passed
@SirLouen
SirLouen deleted the refactor/107-driver-busy-handler branch October 22, 2026 09:03
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.

Drop dbkit/sqlite's own busy handler once modernc.org/sqlite ships its fix

1 participant