Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement: Graceful Handling of Redis LOADING State in Rueidis #656

Closed
nesty92 opened this issue Oct 28, 2024 · 2 comments · Fixed by #657
Closed

Enhancement: Graceful Handling of Redis LOADING State in Rueidis #656

nesty92 opened this issue Oct 28, 2024 · 2 comments · Fixed by #657
Labels
enhancement New feature or request feature good first issue Good for newcomers help wanted Extra attention is needed

Comments

@nesty92
Copy link
Contributor

nesty92 commented Oct 28, 2024

When connecting to a Redis server that is in the process of loading its dataset into memory (e.g., during startup or after a restart), Redis responds to client commands with a LOADING Redis is loading the dataset in memory error. Currently, Rueidis does not handle this state internally, which requires applications to implement custom logic to manage this scenario.

Proposal:

Enhance Rueidis to automatically detect and handle the LOADING state returned by Redis. This could involve:

  • Automatic Retry Mechanism: When a LOADING error is received, Rueidis would retry the command after a configurable delay.
  • Configurable Settings: Allow developers to set retry intervals, maximum retry attempts, and total timeout durations.
  • Exponential Backoff (Optional): Implement an exponential backoff strategy to prevent overwhelming Redis during its loading phase.
  • Error Propagation: If Redis remains in the LOADING state beyond the maximum timeout, Rueidis would return an appropriate error to the application.

Use Case:

In production environments with large datasets, Redis may take significant time to load data from disk into memory. During this period, clients attempting to send commands will receive a LOADING error. Handling this scenario within Rueidis would:

  • Improve Application Resilience: Applications can automatically wait for Redis to become ready without custom retry logic.
  • Reduce Boilerplate Code: Developers won't need to implement repetitive error handling across different applications.
  • Enhance Reliability: Services depending on Redis can recover more gracefully from restarts or failovers.

Current Workaround:

Here's an example of how this issue is currently handled in application code:

ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
defer cancel()
for {
    select {
    case <-ctx.Done():
        return nil, fmt.Errorf("context deadline exceeded while pinging redis: %w", ctx.Err())
    default:
        pingResp := client.Do(ctx,  client.B().Ping().Build())

        if pingResp.Error() == nil {
            return client, nil
        }

        if !strings.Contains(pingResp.Error().Error(), "LOADING Redis is loading the dataset in memory") {
            return nil, pingResp.Error()
        }

        time.Sleep(100 * time.Millisecond)
    }
}

Redis Logs During Loading:

redis-1 | 1:M 28 Oct 2024 10:00:22.750 * DB loaded from incr file appendonly.aof.334.incr.aof: 3.745 seconds
redis-1 | 1:M 28 Oct 2024 10:00:22.750 * DB loaded from append only file: 6.070 seconds
redis-1 | 1:M 28 Oct 2024 10:00:22.750 * Opening AOF incr file appendonly.aof.334.incr.aof on server start

Possible benefits:

  • Seamless Integration: Applications using Rueidis would benefit from built-in handling of Redis's LOADING state.
  • Consistency with Other Clients: Other Redis clients handle this scenario internally; adding this feature would align Rueidis with industry practices.
  • Enhanced Developer Experience: Reduces the need for developers to write custom error handling code for this common scenario.
@rueian rueian added help wanted Extra attention is needed good first issue Good for newcomers enhancement New feature or request feature labels Oct 28, 2024
@rueian
Copy link
Collaborator

rueian commented Oct 28, 2024

Hi @nesty92,

Thanks for opening the issue. I think we can easily make this improvement. We already have Configurable Settings, Exponential Backoff, and Error Propagation (ref). What we don't have is treating the LOADING response as retryable. Would you like to contribute to this?

TODO

  1. Add (r *RedisError) IsLoading() bool below here:

    rueidis/message.go

    Lines 90 to 93 in 6b95467

    // IsTryAgain checks if it is a redis TRYAGAIN message and returns ask address.
    func (r *RedisError) IsTryAgain() bool {
    return strings.HasPrefix(r.string, "TRYAGAIN")
    }

  2. Make (c *singleClient) isRetryable, (c *sentinelClient) isRetryable, and (c *clusterClient) shouldRefreshRetry treat LOADING is retryable.

@nesty92
Copy link
Contributor Author

nesty92 commented Oct 28, 2024

Hi @rueian ,

Thanks for the heads-up and the ideas. I’ll take this on and keep you updated as I make progress. Let me know if there’s anything specific you’d like me to consider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants