lettuce icon indicating copy to clipboard operation
lettuce copied to clipboard

BatchExecutableCommand waits forever if command from batch failed

Open SmirAlex opened this issue 1 year ago • 1 comments

Bug Report

Current Behavior

Test scenario: kill redis server during continuous batch write. Timeout on commands is not set. Behavior: default timeout from connection (1 min) is not applied, application freezes.

Stack trace
// your stack trace here;

Input Code

In this line result value of Futures.await is not checked. As a result, when future fails by timeout, app doesn't react on that and continues waiting. Moreover, even if result would be checked, we would be waited batchSize * defaultTimeout, instead of just defaultTimeout.

Expected behavior/code

Wait default timeout 1 min and throw an exception.

Environment

  • Lettuce version(s): 6.1.6.RELEASE
  • Redis version: 6.0

Possible Solution

Combine all futures from batch into one and check if timeout exceeded.

Additional context

In case when timeout on commands is set through TimeoutOptions, everything works fine. But in my experience, if I set timeout on commands, writing to Redis slows down by 20% - 50% (both simple synchronous write and batch synchronous write). I think this is connected with different logic when timeout is set vs timeout is not set (default connection timeout is applied). So, unfortunately, I must choose between customization and performance. It would be nice, if performance will be the same regardless of whether commands timeout is set or not.

SmirAlex avatar Nov 24 '22 11:11 SmirAlex