No particular reason for this. In another place in our repo, we used 100, how about this?

I think 100 should suffice. Basically, the limit here should be big enough to store all the data chunks that are being queued and are waiting for processing. 100 is a lot for something like this.
Assuming we'll have really tons of data chunks: every data chunk is going to be ~4MB(grpc limit AFAIR).
Here's a communication model for this:
client => backup => blob
So we'd need to be sending far more chunks from the client to the backup than from the backup to the blob at the same time to have this queue really growing.

What do you think?

This is a draft, please ignore all logs and comments, they are for me and are going to be deleted.

This is a draft, please ignore all logs and comments, they are for me and are going to be deleted.

dataChunk is a piece of data, we have to use chunked data because of the grpc limitations - we can at once send up to 4MBof data (this is a constraint that can be changed but we've never done it so far and unless it is really needed I don't see a strong reason to do that. If we one day decide to do it, we'll probably just need to set a new value in a couple of places).

So if we have 1GB of compaction let's say, then we slice it into parts of 4MB and each of these slices is a data chunk.

Can we find a solution that is unbounded?

I'm not sure. We already use MPMCQueue in the WorkerThread and the limit was introduced in https://phabricator.ashoat.com/D1137, later changed in https://phabricator.ashoat.com/D2015 from 20 to 100.

My bet is that if we use this collection with the hardcoded capacity already, then we've probably already checked it and it is not possible to have an unbounded queue. But if you insist I can re-check this.

I quickly checked and MPMCQueue takes Dynamic as a 3rd template argument. The number we pass to the constructor as a capacity specifies how much memory will be allocated up-front. From folly's code:

/// The queue has a fixed capacity, for which all memory will be allocated
/// up front.

also

/// The dynamic version of MPMCQueue allows dynamic expansion of queue
/// capacity, such that a queue may start with a smaller capacity than
/// specified and expand only if needed. Users may optionally specify
/// the initial capacity and the expansion multiplier.

On the other hand

/// *** The dynamic version of MPMCQueue is deprecated. ***
/// Use UnboundedQueue instead.

https://github.com/facebook/folly/blob/18eb46b2a0f03abc2f5d2b673f41b844dc253590/folly/MPMCQueue.h#L118

So I think we have a couple of options here:

1. stay with the bounded MPMCQueue
2. use deprecated dynamic MPMCQueue
3. use UnboundedQueue as suggested by folly's authors

I think option 2 is bad, I'd do 1 or 3. My suggestion is the following: Let's stay with option 1 in this diff with the capacity of 100 which is plenty. I'm going to create a task for this, and we can try using UnboundedQueue. This makes more sense to me, because in this case, we may want to replace using bounded MPMCQueue in the WorkerThread as well.

What do you think?