Skip to content

Conversation

@NitinSingh8
Copy link

Summary

This pull request introduces support for using Redis as a persistence database in Flower. This enhancement allows users to store and retrieve the state data using Redis, providing a more scalable and distributed solution compared to file-based storage.

Key Changes

  • README Update: Added documentation on how to configure Flower to use Redis for persistence, including the necessary command-line arguments.
  • Redis Integration: Modified flower/events.py to support Redis as a backend for state persistence.
    • Added new parameters: redis_host, redis_port, redis_db, redis_key, and redis_ssl.
    • Implemented logic to load and save state data to Redis using redis.StrictRedis.
    • Ensured backward compatibility by maintaining support for file-based persistence using shelve.

Usage

To use Redis as a persistence database, run Flower with the following command-line arguments:

bash
celery -A tasks.app flower --broker=amqp://guest:guest@localhost:5672// --redis_host=localhost --redis_port=6379 --redis_db=0 --redis_key=flower --redis_ssl=True

Benefits

  • Scalability: Redis provides a scalable solution for state persistence, especially in distributed environments.
  • Performance: Redis offers fast read and write operations, improving the performance of state management.
  • Flexibility: Users can choose between Redis and file-based storage based on their requirements.

Testing

  • Verified that state data is correctly saved and loaded from Redis.
  • Ensured that existing functionality with file-based storage remains unaffected.

Notes

  • Ensure that the Redis server is properly configured and accessible from the Flower instance.
  • The default Redis key is set to flower, but it can be customized as needed.

@researcherarjun
Copy link

@NitinSingh8 Does this give an option to store the flower data in redis instead of flower db file? . At present the storing the data in gdbm is creating issues as reported here DB Flower file corrupted

@NitinSingh888
Copy link

@NitinSingh8 Does this give an option to store the flower data in redis instead of flower db file? . At present the storing the data in gdbm is creating issues as reported here DB Flower file corrupted

Yes, it stores the flower data in redis instead of storing flower db file.

help="flower database file")
define("persistent", type=bool, default=False,
help="enable persistent mode")
define("redis_host", type=str, default=None,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason we can't support a redis URL containing all of these params?

@auvipy auvipy requested review from auvipy and Copilot August 12, 2025 17:02
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Redis support as a persistence backend for Flower, allowing users to store and retrieve state data using Redis instead of file-based storage. This provides a more scalable and distributed solution for state persistence.

  • Added Redis configuration options (host, port, database, SSL, key) to command-line arguments
  • Modified Events class to support both Redis and file-based persistence backends
  • Updated application initialization to pass Redis parameters to Events class

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
flower/options.py Adds new command-line options for Redis configuration
flower/events.py Implements Redis persistence logic alongside existing file-based storage
flower/app.py Passes Redis configuration parameters to Events constructor
README.rst Documents Redis usage with example command

self.redis_key = redis_key

# Check if we are using Redis as the database for persistence
self.redis_as_db = self.redis_host and self.redis_port and self.redis_db
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition for enabling Redis persistence is incorrect. When redis_db is 0 (a valid Redis database), the expression evaluates to False, preventing Redis from being used. Change to: self.redis_as_db = self.redis_host and self.redis_port is not None and self.redis_db is not None

Suggested change
self.redis_as_db = self.redis_host and self.redis_port and self.redis_db
self.redis_as_db = self.redis_host and self.redis_port is not None and self.redis_db is not None

Copilot uses AI. Check for mistakes.
state.close()
if self.redis_as_db:
logger.debug("Loading state from Redis...")
redis_client = redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl)
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redis client creation is duplicated in both load and save operations. Consider creating a helper method to avoid code duplication and ensure consistent connection parameters.

Copilot uses AI. Check for mistakes.
Comment on lines +147 to +152
redis_client = redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl)
state_data = redis_client.get(self.redis_key)
redis_client.close()
if state_data:
self.state = pickle.loads(state_data)
logger.debug(f"State loaded from Redis: {self.state}")
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redis connections should be properly managed using context managers or try/finally blocks to ensure cleanup even if exceptions occur. Consider using with redis.Redis(...) as redis_client: or wrap in try/finally.

Suggested change
redis_client = redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl)
state_data = redis_client.get(self.redis_key)
redis_client.close()
if state_data:
self.state = pickle.loads(state_data)
logger.debug(f"State loaded from Redis: {self.state}")
with redis.Redis(host=self.redis_host, port=self.redis_port, db=self.redis_db, ssl=self.redis_ssl) as redis_client:
state_data = redis_client.get(self.redis_key)
if state_data:
self.state = pickle.loads(state_data)
logger.debug(f"State loaded from Redis: {self.state}")

Copilot uses AI. Check for mistakes.
redis_client.close()
if state_data:
self.state = pickle.loads(state_data)
logger.debug(f"State loaded from Redis: {self.state}")
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logging the entire state object may expose sensitive information. Consider logging only a summary or removing this debug statement to prevent potential data leakage.

Suggested change
logger.debug(f"State loaded from Redis: {self.state}")
logger.debug(f"State loaded from Redis: type={type(self.state).__name__}, event_count={len(getattr(self.state, 'events', []))}")

Copilot uses AI. Check for mistakes.
Comment on lines +224 to +226
state_data = pickle.dumps(self.state)
redis_client.set(self.redis_key, state_data)
redis_client.close()
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as the load operation, Redis connections should be properly managed using context managers or try/finally blocks to ensure cleanup even if exceptions occur.

Suggested change
state_data = pickle.dumps(self.state)
redis_client.set(self.redis_key, state_data)
redis_client.close()
try:
state_data = pickle.dumps(self.state)
redis_client.set(self.redis_key, state_data)
finally:
redis_client.close()

Copilot uses AI. Check for mistakes.

In this example, Flower is using the `tasks.app` defined in the `examples/tasks.py <https://github.com/mher/flower/blob/master/examples/tasks.py>`_ file

## Use Redis as a database for Persistence
Copy link

Copilot AI Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The heading uses Markdown syntax (##) in a reStructuredText file. Use reStructuredText syntax instead: a title followed by equal signs or dashes on the next line.

Suggested change
## Use Redis as a database for Persistence
Use Redis as a database for Persistence
---------------------------------------

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants