Skip to content

sqlite: reject non-positive backup rates - #64893

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:sqlite-backup-settle-rate-0
Open

sqlite: reject non-positive backup rates#64893
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:sqlite-backup-settle-rate-0

Conversation

@trivikr

@trivikrtrivikr commented Aug 1, 2026

Copy link
Copy Markdown
Member

Fixes: #64892

Passing a rate of 0 to backup() causes sqlite3_backup_step() to copy no pages. The backup job then continually reschedules itself and the returned promise never settles.

This change requires backup rates to be positive integers to prevent zero-work backup jobs.

Assisted-by: codex:gpt-5.6-sol

@trivikr
sqlite: reject non-positive backup rates
d666b94
Passing a rate of 0 to backup() causes sqlite3_backup_step() to copy
no pages. The backup job then continually reschedules itself and the
returned promise never settles.
Require backup rates to be positive integers to prevent zero-work
backup jobs.
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: codex:gpt-5.6-sol
@nodejs-github-bot

nodejs-github-bot commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/sqlite

@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. sqlite Issues and PRs related to the SQLite subsystem. labels Aug 1, 2026
@trivikrtrivikr mentioned this pull request Aug 1, 2026
Open
@trivikrtrivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 1, 2026
@codecov

codecovBot commented Aug 1, 2026•
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.15%. Comparing base (8a1ca0f) to head (d666b94).
⚠️ Report is 34 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #64893 +/- ##
==========================================
+ Coverage 90.14% 90.15% +0.01% 
==========================================
Files 746 746 Lines 242849 242851 +2 Branches 45771 45768 -3 ==========================================
+ Hits 218906 218939 +33 + Misses 15438 15390 -48 - Partials 8505 8522 +17
Files with missing linesCoverage Δ
src/node_sqlite.cc80.92% <100.00%> (+0.03%)⬆️

... and 40 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.

araujogui
araujogui approved these changes Aug 2, 2026
View reviewed changes

@araujoguiaraujogui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 2, 2026
@nodejs-github-bot

This comment was marked as outdated.

Sign in to view
@nodejs-github-bot

nodejs-github-bot commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/75399/

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnelljasnelljasnell approved these changes
+1 more reviewer
@araujoguiaraujoguiaraujogui approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.sqliteIssues and PRs related to the SQLite subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sqlite: backup() never settles when rate is 0

4 participants

@trivikr@nodejs-github-bot@jasnell@araujogui