Agent-Hellboy / Agent-Hellboy/gunicorn-prometheus-exporter
Redundant Redis Storage Implementation in PR #82 - Should be Discarded
- Vorherrschende Sprache
- Python
- Sterne
- 18
- Forks
- 1
- PR-Merge-Kennzahlen
- Keine gemergten PRs in 30 T.
Beschreibung
## Problem
PR #82 contains a diff that adds a new file `src/gunicorn_prometheus_exporter/storage/redis_backend/storage_dict.py` with a `RedisDict` class. However, this implementation is **redundant** and **inferior** to the existing Redis storage functionality already present in the current codebase.
## Current Superior Implementation
The codebase already has a production-ready `RedisStorageDict` class in:
`src/gunicorn_prometheus_exporter/backend/core/client.py`
## Critical Issues in Proposed Implementation
### 1. **Blocking Redis Operations** 🚨
- Uses `redis.keys()` which blocks Redis server
- Current implementation uses `scan_iter()` (non-blocking)
### 2. **Type Handling Bugs** 🐛
- Attempts string operations on bytes objects
- Missing proper bytes-to-string conversion
- Will cause TypeError in production
### 3. **Missing Enterprise Features** ❌
- No TTL (Time-To-Live) support → Redis memory will grow indefinitely
- No process isolation → worker metrics will conflict
- No error handling → Redis failures will crash the application
- No logging/monitoring → silent failures
- No cleanup capabilities → dead worker metrics accumulate forever
## Comparison
| Feature | Current Implementation | Proposed Diff | Risk Level |
|---------|----------------------|---------------|------------|
| Redis Performance | ✅ Non-blocking `scan_iter` | ❌ Blocking `keys()` | **CRITICAL** |
| Memory Management | ✅ TTL support | ❌ No TTL | **HIGH** |
| Multi-process Safety | ✅ PID isolation | ❌ Shared namespace | **HIGH** |
| Error Handling | ✅ Graceful degradation | ❌ Hard crashes | **CRITICAL** |
| Process Cleanup | ✅ Automated cleanup | ❌ Manual only | **HIGH** |
## Recommendation
**Discard the entire diff for `storage/redis_backend/storage_dict.py`** because:
1. It duplicates existing functionality
2. It introduces production-breaking bugs
3. It uses an outdated file structure
4. The current implementation is already superior and production-tested
## Links
- **Current PR**: #82
- **Existing Implementation**: `src/gunicorn_prometheus_exporter/backend/core/client.py` (lines ~80-270)
- **PR Review Comment**: https://github.com/Agent-Hellboy/gunicorn-prometheus-exporter/pull/82#discussion_r2350338369
## Action Items
- [ ] Remove the proposed `storage/redis_backend/storage_dict.py` file from PR #82
- [ ] Keep the existing `backend/core/client.py` implementation
- [ ] Update any references to use the current backend structure
- [ ] Consider adding tests for the existing implementation if missing
Beitragsleitfaden
Bewertung
Dieses Issue wurde noch nicht bewertet.