Protect against doing requests for snapshots that are over 400KB. #13
Conversation
|
Nice! To clarify: This is to improve the error handling in the case a snapshot would be rejected due to exceeding DynamoDB's maximum item size? I also like that this is part of the code that would be required to dispatch large snapshots to S3 :) |
| @@ -12,6 +12,8 @@ import com.typesafe.config.Config | |||
| import akka.persistence.dynamodb._ | |||
| import scala.concurrent.Future | |||
|
|
|||
| class DynamoDBSnapshotRejection(message: String, cause: Throwable = null) extends RuntimeException(message, cause) | |||
There was a problem hiding this comment.
Can we add a Key field as a val to this exception so we know which key caused the exception incase people want to catch it.
Also mention the key in the exception message.
There was a problem hiding this comment.
Seems like that then breaks consistency with e.g.
https://github.com/esko-oss/pekko-persistence-dynamodb/blob/4d172134208279a00b90ef597dd0970c31b56fde/src/main/scala/akka/persistence/dynamodb/journal/DynamoDBJournal.scala#L30
If this exception ripples back to the persistent actor, the persistence ID would be known there too.
| * https://zaccharles.medium.com/calculating-a-dynamodb-items-size-and-consumed-capacity-d1728942eb7c | ||
| * https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/CapacityUnitCalculations.html | ||
| */ | ||
| val dynamoFixedByteSize = |
There was a problem hiding this comment.
Should be DynamoFixedByteSize since its a constant (Scala convention).
There was a problem hiding this comment.
We'll adapt when we rebase this.
It is, avoiding a sizeable round trip in doing so. |
spangaer
force-pushed
the
feature/snapshot-size
branch
from
4d17213
to
74cd88f
Compare
src/test/scala/org/apache/pekko/persistence/dynamodb/snapshot/SnapshotTooBigSpec.scala
Show resolved
Hide resolved
spangaer
force-pushed
the
feature/snapshot-size
branch
from
74cd88f
to
a44c4ae
Compare
|
The headers should be fixed, but it looks like we're still doing something wrong with the formatting. |
|
@spangaer SnapshotTooBigSpec has 2 formatting issues
|
spangaer
force-pushed
the
feature/snapshot-size
branch
from
a44c4ae
to
58fcd9b
Compare
|
Should be fixed now. |
@mdedetrich could you re-review? I've created a 1.0.x branch so it should now be safe to merge 1.1 stuff. |
Sources:
https://zaccharles.medium.com/calculating-a-dynamodb-items-size-and-consumed-capacity-d1728942eb7c https://docs.aws.amazon.com/amazondynamodb/latest/developerguide/CapacityUnitCalculations.html
Draft note
I'm well aware that the rename refactor and deployment processes are still ongoing. We'll adapt these commits to the new reality as it materializes but wanted to stage the contributions already.
Also FYI @coreyoconnor