mirrorbits icon indicating copy to clipboard operation
mirrorbits copied to clipboard

Draft: Split MULTI transaction in batches (fix #149)

Open elboulangero opened this issue 2 years ago • 3 comments

Unbounded MUTLTI transactions can cause big latency spikes in Redis. Especially when the number of commands in the transaction is a factor of the number of files in the repository, and the repository might have up to a million file.

With this MR, we make sure that the MULTI transactions are executed in batches of 5k files, following the recommendation from https://redis.io/docs/manual/pipelining/:

IMPORTANT NOTE: While the client sends commands using pipelining, the server will be forced to queue the replies, using memory. So if you need to send a lot of commands with pipelining, it is better to send them as batches each containing a reasonable number, for instance 10k commands, read the replies, and then send another 10k commands again, and so forth. The speed will be nearly the same, but the additional memory used will be at most the amount needed to queue the replies for these 10k commands.

elboulangero avatar Oct 11 '23 14:10 elboulangero

Is that ready ?

jbkempf avatar Feb 12 '25 08:02 jbkempf

It's not in fantastic shape, I'd need to do another pass on it before calling it ready.

At the same time, I'm testing mirrorbits with valkey (a fork of redis), and valkey has multi-thread support. I wonder if that could fix the latency spikes issues, and in that case that would render this MR obsolete. I hope I can answer this question by next week.

elboulangero avatar Feb 12 '25 14:02 elboulangero

It's not in fantastic shape, I'd need to do another pass on it before calling it ready.

At the same time, I'm testing mirrorbits with valkey (a fork of redis), and valkey has multi-thread support. I wonder if that could fix the latency spikes issues, and in that case that would render this MR obsolete. I hope I can answer this question by next week.

Then, let's skip that one for now. We don't need that. We (you mostly) have done enough for this release.

jbkempf avatar Feb 12 '25 19:02 jbkempf