akka-persistence-tck update to include a new test case for plugins #21706
Conversation
|
Can one of the repo owners verify this patch? |
|
OK TO TEST |
|
Isn't the limit of 10k just as arbitrary as the 4k limit you stumbled upon? |
|
Yes, it is quite arbitrary, but I stumbled upon 4K. I can change it 4k if you want, but this is oracle specific. We could try bigger snapshot actually. |
|
Test PASSed. |
|
I suggest to place the limit in a method that can be overridden. 10k sounds like a good default requirement |
98d4b57 to
0a9ea16
Compare
|
Extracted the limit into the method. |
|
Oh just realised I made it 10000, shall I change it to 10 * 1024 ? |
|
Test PASSed. |
| s"save bigger size snapshot ($snapshotByteSizeLimit bytes)" in { | ||
| val metadata = SnapshotMetadata(pid, 100) | ||
| val bigSnapshot = for { i ← 1 to snapshotByteSizeLimit } yield '0' | ||
| snapshotStore.tell(SaveSnapshot(metadata, bigSnapshot.mkString("")), senderProbe.ref) |
There was a problem hiding this comment.
a more compact way of creating the String:
"0" * snapshotByteSizeLimit
0a9ea16 to
4654e17
Compare
|
Test PASSed. |
|
I'm sorry for bumping in, but as we had similar problems with persistence (cassandra backend to be precise) I see it being a dead path. I've ended up writing our custom cassandra persistence impl that splits payload into chunks of configurable size to overcome cassandra limitations. That said, perhaps, it makes sense to reconsider the schema and persistence jdbc impl. And also there is no limit for message size documented for persistence (of course, common sense etc), so this looks like an artificial limitation, imho. |
|
Having this integration test as part of akka-persistence-tck package will highlight the limit explicitly, and if test fails for some of the implementations, you might always override the limit for your plugin and then make a note in your plugin documentation that it supports only X bytes snapshots. |
|
exactly, that is the purpose of the tck - many things will be optional (but they should not be accidental) |
| @@ -52,6 +52,8 @@ abstract class SnapshotStoreSpec(config: Config) extends PluginSpec(config) | |||
| } | |||
| } | |||
|
|
|||
| def snapshotByteSizeLimit = 10000 | |||
There was a problem hiding this comment.
Maybe add a scaladoc on this mentioning that it can be overridden to a limit suitable for the specific implementation but that authors should also document that?
… with big snapshots.
4654e17 to
12a2ca6
Compare
|
Test PASSed. |
|
Thanks @skisel ! |
PR to fix the problem mentioned in #21704