fix: Database dumps do not work on large databases (#59)#218
Open
IbrahimLaeeq wants to merge 1 commit into
Open
fix: Database dumps do not work on large databases (#59)#218IbrahimLaeeq wants to merge 1 commit into
IbrahimLaeeq wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes the /export/dump endpoint so large databases can be exported without exhausting memory or hitting the ~30s request window. The handler now streams output via a ReadableStream and reads each table in bounded LIMIT/OFFSET pages instead of buffering the entire database into a single string.
Changes:
- Replace the in-memory
dumpContentstring +Blobresponse with aReadableStreamthat enqueues schema andINSERTstatements as they are generated. - Paginate per-table data reads with
LIMIT 1000 OFFSET n, stopping when a short page is returned. - Add a test verifying that a 1000-row full page triggers a follow-up
OFFSET 1000query and that pagination usesLIMIT/OFFSET.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/export/dump.ts | Switches the dump response body to a ReadableStream and paginates per-table data via LIMIT/OFFSET. |
| src/export/dump.test.ts | Adds a pagination test asserting four executeOperation calls and correct LIMIT/OFFSET usage across pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+62
to
+70
| const dataResult = await executeOperation( | ||
| [ | ||
| { | ||
| sql: `SELECT * FROM ${table} LIMIT ${DUMP_PAGE_SIZE} OFFSET ${offset};`, | ||
| }, | ||
| ], | ||
| dataSource, | ||
| config | ||
| ) |
Comment on lines
+97
to
+100
| } catch (error: any) { | ||
| console.error('Database Dump Error:', error) | ||
| controller.error(error) | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #59.
/claim #59
emory at once.
Both also meant nothing was sent to the client until the entire dump finished, so any dump exceeding the ~30s request window failed with no partial output.
Fix (
src/export/dump.ts):ReadableStream. Each schema line andINSERTstatement isenqueued as it's generated, so memory stays flat regardless of database size and bytes start flowing to the client immediately — keeping the connection alive well past the 30s window for large dumps.LIMIT 1000 OFFSET n, looping until a short page signals end-of-table, so no single large table is ever fully held in memory.500response; errors during streaming go throughcontroller.error.Test (
src/export/dump.test.ts): Added a case that mocks a full 1000-row page followed by a partial page, asserting the dump paginates (4executeOperationcalls) and issuesLIMIT/OFFSET 0thenOFFSET 1000queries. All existing tests still pass unchanged since streamed responses are still readable viaresponse.text().Note: this is the focused, dependency-free fix. The issue's fuller "proposed solution" (R2 binding, DO alarms, callback URLs) is a much larger feature requiring new infrastructure bindings and config; the streaming + pagination change is the smallest correct fix that removes the memory ceiling and the hard timeout failure.
Verified against the repository's own test suite before submission.