Marshal values before batch writing

kitar / laravel-dynamodb

A DynamoDB based Eloquent model and Query builder for Laravel.

MIT License

179 stars 27 forks source link

Marshal values before batch writing #35

Closed negoziator closed 1 year ago

negoziator commented 1 year ago

NOT TESTET

This is pretty hard to test. Yesterday i changed our internal systems to use the kitar version of batch write items, instead of our own implementation. We are doing aprox 50.000 writes a day, and we began to see some errors where the structure is not valid for batch write items. So i compared our solution wiith the one in Kitar - and the only difference i can see is that we were marshalling.

Here' the error we got:

Left this as a draft - to start a coversation 😄

kitar commented 1 year ago

Thanks for reporting! I’ll look at more detail later, but for now, I’ll share this with you:

In this library, Marshaling is performed in Grammer
Batch operations should also be Marshaled
For example, batchPutItem batchDeleteItem batchWriteItem will be marshaled here
- https://github.com/kitar/laravel-dynamodb/blob/380b7aaaf4b6c18648eb4b35287eed43336c3ccd/src/Kitar/Dynamodb/Query/Grammar.php#L160-L184

kitar commented 1 year ago

There may be cases where it is not marshaled correctly😅 If possible, please share some example params which you pass to batchPutItem or batchWriteItem.

negoziator commented 1 year ago

@kitar Ahh. You are completely right. Sorry. I'll close this again, and see if i can find more details about when this happens, and then come back 😄

negoziator commented 1 year ago

I don't know if you can use this for anything .. but on the left was my version of it. On the right is yours.

I will have to revert back to mine now - because we are seeing alot of these errors now - i will have to try and see if i can trigger the same error on our staging environment 😄

negoziator commented 1 year ago

@kitar just fyi: after my revert it instantly stopped with the errors, At this moment i'm really not sure why it happens.

Sorry for taking your time - when i have so little information about it.

kitar commented 1 year ago

@negoziator No problem at all! If you have any other concerns, feel free to let me know and I'd appreciate it😄

It's just a possibility, but as far as this commit is concerned, maybe you were double Marshal.

FYI, you can also debug the parameters that are actually sent to DynamoDB by adding dryRun() when querying. I’ll document about this somewhere.