Page MenuHomePhabricator

[services] Backup - Fix dynamoDB item size limit
ClosedPublic

Authored by karol on Jun 22 2022, 3:31 AM.
Tags
None
Referenced Files
F2907809: D4324.id14346.diff
Sun, Oct 6, 12:21 PM
F2907798: D4324.id14317.diff
Sun, Oct 6, 12:17 PM
F2907758: D4324.id13932.diff
Sun, Oct 6, 12:01 PM
Unknown Object (File)
Fri, Oct 4, 4:01 AM
Unknown Object (File)
Wed, Oct 2, 5:28 AM
Unknown Object (File)
Fri, Sep 27, 11:47 AM
Unknown Object (File)
Fri, Sep 27, 6:16 AM
Unknown Object (File)
Wed, Sep 25, 11:38 AM

Details

Summary

Depends on D4323

linear task: https://linear.app/comm/issue/ENG-1288/properly-handle-the-dynamodb-item-size-limit

Finally, the proper value that is 400KiB (400 * 1024) (not KB) based on the research described in the linked task.

Test Plan

-

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Jun 22 2022, 11:25 AM
karol edited the summary of this revision. (Show Details)

rebase

There's a lot more going on here than when I initially accepted. Would love if @palys-swm could take a pass

This revision now requires review to proceed.Jun 30 2022, 6:49 AM
tomek added inline comments.
services/commtest/tests/backup_test.rs
47 ↗(On Diff #14033)

Please reformat the code - some lines seem to be too long

134 ↗(On Diff #14033)

I suggested this somewhere: we should avoid magic numbers like this and instead give them meaningful names. Also, if the variables depend on each other, one should be computed based on the other.

153 ↗(On Diff #14033)

assert_eq

This revision is now accepted and ready to land.Jun 30 2022, 7:28 AM
services/commtest/tests/backup_test.rs
134 ↗(On Diff #14033)

this is resolved in D4326 these changes are going to be landed together anyway

This revision was landed with ongoing or failed builds.Jul 8 2022, 2:31 AM
This revision was automatically updated to reflect the committed changes.