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

Jump to bottom

Failed to upload backup: too large #1719

Closed
bonomat opened this issue Dec 11, 2023 · 13 comments · Fixed by #1837
Closed

Failed to upload backup: too large #1719

bonomat opened this issue Dec 11, 2023 · 13 comments · Fixed by #1837
Labels
bug Something isn't working

Comments

@bonomat
Copy link
Contributor

bonomat commented Dec 11, 2023

just noticed locally:

flutter: [D] TIME: 2023-12-11T07:39:51.686165Z r: Response status code 413 Payload Too Large
flutter: [E] TIME: 2023-12-11T07:39:51.686640Z r: Failed to upload backup. Failed to buffer the request body: length limit exceeded
@bonomat bonomat added the bug Something isn't working label Dec 11, 2023
@bonomat
Copy link
Contributor Author

bonomat commented Dec 11, 2023 •
edited
Loading

Happens continuesly now

@holzeis
Copy link
Contributor

holzeis commented Dec 11, 2023

I think the limit is currently set to 20mb? Do you know what file did get so large?

@bonomat
Copy link
Contributor Author

bonomat commented Dec 12, 2023

Needs to be investigated more. The logs are gone now so I don't know which file was too big.

@holzeis
Copy link
Contributor

holzeis commented Dec 12, 2023

related to #1637

@holzeis
Copy link
Contributor

holzeis commented Dec 13, 2023

related to #1737

@github-actions GitHub Actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Jan 13, 2024
@github-actions github-actions bot removed the Stale label Jan 18, 2024
@luckysori
Copy link
Contributor

luckysori commented Jan 18, 2024 •
edited
Loading

The app_can_be_restored_from_a_backup e2e test fails with this error now.

I think this must have to do with #1817, since we potentially added lots of CET adaptor signatures to the app's database. If that's the problem, how should we fix it, @holzeis?

luckysori added a commit that referenced this issue Jan 18, 2024
@luckysori
fix(coordinator): Allow requests with up to 50MB bodies
2dccdd8 
By default this is set to 2MB, according to the docs[1].

We need larger bodies than before (even in the e2e test!), because we
added lots of CET adaptor signatures to the database recently[2].

I also played around with nginx's `client_max_body_size`, but that's
actually irrelevant since we don't use nginx as a reverse proxy for
the coordinator in the e2e tests.

Related to #1719.

[1]: https://docs.rs/axum/latest/axum/extract/struct.DefaultBodyLimit.html#method.disable.
[2]: #1817.
@luckysori luckysori mentioned this issue Jan 18, 2024
Merged
@holzeis
Copy link
Contributor

holzeis commented Jan 18, 2024

The app_can_be_restored_from_a_backup e2e test fails with this error now.

I think this must have to do with #1817, since we potentially added lots of CET adaptor signatures to the app's database. If that's the problem, how should we fix it, @holzeis?

Hmm, we might be able to tell SQLite to skip a specific table for backup. We don't need to backup the last_outbound_dlc_messages table.

@holzeis
Copy link
Contributor

holzeis commented Jan 18, 2024

Hmm, we might be able to tell SQLite to skip a specific table for backup. We don't need to backup the last_outbound_dlc_messages table.

It looks like there is no way telling sqlite to backup only some tables, but its possible to backup only a specific database. So we could add a second database for the non-critical large data, which does not get backed up.

@holzeis
Copy link
Contributor

holzeis commented Jan 18, 2024

@luckysori reopening that issue, as I don't think you meant to close it?

@holzeis holzeis reopened this Jan 18, 2024
@luckysori
Copy link
Contributor

luckysori commented Jan 18, 2024 •
edited
Loading

@luckysori reopening that issue, as I don't think you meant to close it?

I suppose the fix that we introduced might be enough if a backup chunk never exceeds 50MB (ignoring the fact that the proxy limit is set to 20MB).

But a 50MB chunk is already massive and we definitely want to bring that down.

@bonomat
Copy link
Contributor Author

bonomat commented Jan 18, 2024

@luckysori reopening that issue, as I don't think you meant to close it?

He actually did fix it here: 2dccdd8

@holzeis
Copy link
Contributor

holzeis commented Jan 18, 2024

Oh nice, I've missed that 🙂

@holzeis holzeis closed this as completed Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Another fix for the e2e tests get10101/10101
3 participants
@bonomat @holzeis @luckysori