Details

services/tunnelbroker/src/Database/DatabaseManager.cpp
171–194 ↗	(On Diff #13894)	Could you find a way to avoid the duplication between this and `putMessageItem`?
199–201 ↗	(On Diff #13894)	According to the documentation https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_BatchWriteItem.html The individual PutItem and DeleteItem operations specified in BatchWriteItem are atomic; however BatchWriteItem as a whole is not. If any requested operations fail because the table's provisioned throughput is exceeded or an internal processing failure occurs, the failed operations are returned in the UnprocessedItems response parameter. You can investigate and optionally resend the requests. Typically, you would call BatchWriteItem in a loop. Each iteration would check for unprocessed items and submit a new BatchWriteItem request with those unprocessed items until all items have been processed. especially Typically, you would call BatchWriteItem in a loop. Each iteration would check for unprocessed items and submit a new BatchWriteItem request with those unprocessed items until all items have been processed. Can we have something like that? Probably `innerBatchWriteItem` should be modified.

This revision now requires changes to proceed.Jun 29 2022, 5:01 AM

• max planned changes to this revision.Jun 29 2022, 1:38 PM

Using a private function to populate put requests in a one place instead of duplicating it.

services/tunnelbroker/src/Database/DatabaseManager.cpp
171–194 ↗	(On Diff #13894)	Could you find a way to avoid the duplication between this and `putMessageItem`? I've added a private function to populate the `PutRequest` and `PutItemRequest` in one place.
199–201 ↗	(On Diff #13894)	According to the documentation https://docs.aws.amazon.com/amazondynamodb/latest/APIReference/API_BatchWriteItem.html The individual PutItem and DeleteItem operations specified in BatchWriteItem are atomic; however BatchWriteItem as a whole is not. If any requested operations fail because the table's provisioned throughput is exceeded or an internal processing failure occurs, the failed operations are returned in the UnprocessedItems response parameter. You can investigate and optionally resend the requests. Typically, you would call BatchWriteItem in a loop. Each iteration would check for unprocessed items and submit a new BatchWriteItem request with those unprocessed items until all items have been processed. especially Typically, you would call BatchWriteItem in a loop. Each iteration would check for unprocessed items and submit a new BatchWriteItem request with those unprocessed items until all items have been processed. Can we have something like that? Probably `innerBatchWriteItem` should be modified. `innerBatchWriteItem` in D4373 was modified according to this issue.

tomek added inline comments.

services/tunnelbroker/src/Database/DatabaseManager.cpp
138–142		It's probably possible to simplify the usage a bit by: creating a new instance of `T` in this function specialize based only on the return type It would only work when `T` have a default constructor, but it's still worth considering
140		We shouldn't start a parameter name with uppercase letter
171–194 ↗	(On Diff #13894)	Great!

This revision is now accepted and ready to land.Jul 1 2022, 3:43 AM

Rebase on the stack updates (innerBatchWriteItem function), fixed parameter name.

services/tunnelbroker/src/Database/DatabaseManager.cpp
138–142	It would only work when `T` have a default constructor, but it's still worth considering. Yes, I was thinking about such an approach too, but not sure it's worth it here. Thanks for the suggestion!
140	We shouldn't start a parameter name with uppercase letter Fixed, thanks!