Details

services/tunnelbroker/src/Database/DatabaseManager.cpp
171–194	Could you find a way to avoid the duplication between this and `putMessageItem`?
199–201	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	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	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 ↗	(On Diff #14030)	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 ↗	(On Diff #14030)	We shouldn't start a parameter name with uppercase letter
171–194		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 ↗	(On Diff #14030)	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 ↗	(On Diff #14030)	We shouldn't start a parameter name with uppercase letter Fixed, thanks!