Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sqlite: expose backup api #56253

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

sqlite: expose backup api #56253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
666034f
sqlite, test: expose sqlite online backup api
geeksilva97 Jan 13, 2025
c73a3cc
sqlite: resolve backup promise with total transferred pages
geeksilva97 Jan 13, 2025
5ad3f48
sqlite,test: test DatabaseSync.prototype.backup interface
geeksilva97 Jan 13, 2025
a050cd6
sqlite: pass object to progress function instead of two args
geeksilva97 Jan 14, 2025
9d8538b
fixup: improve tests
geeksilva97 Jan 14, 2025
4abb1b2
sqlite: update interface
geeksilva97 Jan 14, 2025
74a9ad4
doc: add backup docs
geeksilva97 Jan 14, 2025
d58fd03
test: use temp dir to test invalid paths
geeksilva97 Jan 14, 2025
e3e7431
doc: fixup: fix typos
geeksilva97 Jan 15, 2025
acec23e
fixup: improve formatting
geeksilva97 Jan 16, 2025
fa23c05
fixup: centralize backup error handling
geeksilva97 Jan 16, 2025
c532c58
fixup: fix new env properties order
geeksilva97 Jan 20, 2025
0df678e
fixup: await rejects assertion
geeksilva97 Jan 20, 2025
7b15cae
sqlite,test,doc: move backup implementation to a separate function
geeksilva97 Jan 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions doc/api/sqlite.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,27 @@ exception.
| `TEXT` | {string} |
| `BLOB` | {Uint8Array} |

## `sqlite.backup(sourceDb, destination[, options])`

<!-- YAML
added: REPLACEME
-->

* `sourceDb` {DatabaseSync} The database to backup. The source database must be open.
* `destination` {string} The path where the backup will be created. If the file already exists, the contents will be
Copy link
Member

Choose a reason for hiding this comment

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

If this is assumed to be a filesystem path, then handling it the same as our other fs APIs would be ideal. Paths can be expressed as either strings, Buffer, or file:// scheme URLs. If you're not familiar, there are internal utilities for normalizing these into a usable path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense. I will search for examples to see how to do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe that? https://github.com/nodejs/node/blob/main/lib/internal/fs/utils.js#L700-L720

jasnell marked this conversation as resolved.
Show resolved Hide resolved
overwritten.
* `options` {Object} Optional configuration for the backup. The
following properties are supported:
* `source` {string} Name of the source database. **Default:** `'main'`.
* `target` {string} Name of the target database. **Default:** `'main'`.
Copy link
Member

Choose a reason for hiding this comment

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

For users who may not be that familiar with things here, the docs could likely benefit from more explanation about how source and target are used.

* `rate` {number} Number of pages to be transmitted in each batch of the backup. **Default:** `100`.
* `progress` {Function} Callback function that will be called with the number of pages copied and the total number of
Copy link
Member

Choose a reason for hiding this comment

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

Not something that needs to be added in this PR but... would it make sense for this operation to support cancelation via an AbortSignal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

pages.
* Returns: {Promise} A promise that resolves when the backup is completed and rejects if an error occurs.

This method makes a database backup. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][]
and [`sqlite3_backup_finish()`][] functions.
Copy link
Member

Choose a reason for hiding this comment

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

Since the backup operation is async and may take some time, some discussion in here about concurrent edits while the db is being backed up would be helpful for users to understand what to expect.


## `sqlite.constants`

<!-- YAML
Expand Down Expand Up @@ -604,6 +625,9 @@ resolution handler passed to [`database.applyChangeset()`][]. See also
[`SQLITE_DETERMINISTIC`]: https://www.sqlite.org/c3ref/c_deterministic.html
[`SQLITE_DIRECTONLY`]: https://www.sqlite.org/c3ref/c_deterministic.html
[`database.applyChangeset()`]: #databaseapplychangesetchangeset-options
[`sqlite3_backup_finish()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupfinish
[`sqlite3_backup_init()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupinit
[`sqlite3_backup_step()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupstep
[`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
[`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
[`sqlite3_create_function_v2()`]: https://www.sqlite.org/c3ref/create_function.html
Expand Down
4 changes: 4 additions & 0 deletions src/env_properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@
V(asn1curve_string, "asn1Curve") \
V(async_ids_stack_string, "async_ids_stack") \
V(attributes_string, "attributes") \
V(backup_string, "backup") \
V(base_string, "base") \
V(bits_string, "bits") \
V(block_list_string, "blockList") \
Expand Down Expand Up @@ -293,6 +294,7 @@
V(primordials_string, "primordials") \
V(priority_string, "priority") \
V(process_string, "process") \
V(progress_string, "progress") \
V(promise_string, "promise") \
V(prototype_string, "prototype") \
V(psk_string, "psk") \
Expand All @@ -306,6 +308,7 @@
V(reason_string, "reason") \
V(refresh_string, "refresh") \
V(regexp_string, "regexp") \
V(remaining_pages_string, "remainingPages") \
V(rename_string, "rename") \
V(replacement_string, "replacement") \
V(required_module_facade_url_string, \
Expand Down Expand Up @@ -357,6 +360,7 @@
V(time_to_first_byte_sent_string, "timeToFirstByteSent") \
V(time_to_first_header_string, "timeToFirstHeader") \
V(tls_ticket_string, "tlsTicket") \
V(total_pages_string, "totalPages") \
V(transfer_string, "transfer") \
V(transfer_unsupported_type_str, \
"Cannot transfer object of unsupported type.") \
Expand Down
Loading
Loading