Adding concurrent batch writes

[davidjumani]

Added RunConcurrently() to concurrently execute BatchWriteItem()

[greggjs]

I have no power over if this gets merged or not, I'm just waiting for PR in the same repo to get merged myself. I thought I'd provide some feedback on your PR. I think you need some big changes to it if you want this thing merged.

[greggjs]

Why do you need this variable when you could just do

for _, batch := range totalBatch {
        go bw.writeBatch(ctx, batch, channel)
}

and I don't think you even need the batch counter. It doesn't really seem like you're using it for any reason.

[davidjumani]

Was using this for testing and debugging. Forgot to remove it!

[greggjs]

Would you rather use some kind of multi error thing here? To me you could have multiple errors that occur from all your batches that you'd want to record. I'd suggest something like hashicorp/go-multierror

[greggjs]

I think you don't need to do it this way. I'd think a better way to do this would be

for {
        select {
        case batchResponse, ok := <- channel:
                // logic for handing errors / unexpected stream close / updating written records
                // when all batches responded, return the wrote count.
        case <-ctx.Done():
                // do some error logic because we timed out the ctx, return the currently recorded wrote records
        }
}

[davidjumani]

Will try this. However the final writeBatch does not close the channel. Would it suffice to count and break within the case or is there a smarter way about it??

[greggjs]

If you take those suggestions, you could get rid of this outer for loop. and just have those two.

[greggjs]

I would make this a deferred operation, as I wouldn't want to prematurely close the channel. I think with your current code this is a real possibility.

[greggjs]

You're not really using the notion of your threads really. I think it's overly complicating your operation as it's currently implemented. All you are really doing here is spinning up a go routine for each batch you're running, and you'll make too many operation if your threads number is larger than your total batches.

If you were truly concerned about how many go routines were concurrently running, you'd be making a worker pool, like https://gobyexample.com/worker-pools and send your batches to be processed by the worker pool. I'd also not call these things "Threads" as a common Go convention would be to refer to it as "Pool Size".

[davidjumani]

Will try this. Thanks!

[greggjs]

Don't use the word thread, use pool size.

[davidjumani]

@greggjs Thanks for your suggestions. I've made the changes! Let me know what you think!