-
Notifications
You must be signed in to change notification settings - Fork 36
⚡ Bolt: Optimized system-wide caching and implemented performant blockchain integrity for grievances #559
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
base: main
Are you sure you want to change the base?
⚡ Bolt: Optimized system-wide caching and implemented performant blockchain integrity for grievances #559
Changes from all commits
b564e31
24eb07c
869ac0b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,12 +5,14 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| import json | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import uuid | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| import hashlib | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Dict, Any, Optional, List | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from sqlalchemy.orm import Session, joinedload | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from datetime import datetime, timezone, timedelta | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.models import Grievance, Jurisdiction, GrievanceStatus, SeverityLevel, Issue | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.database import SessionLocal | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.cache import grievance_last_hash_cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.routing_service import RoutingService | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.sla_config_service import SLAConfigService | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.escalation_engine import EscalationEngine | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -84,6 +86,22 @@ def create_grievance(self, grievance_data: Dict[str, Any], db: Session = None) - | |||||||||||||||||||||||||||||||||||||||||||||||||||
| # Generate unique ID | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| unique_id = str(uuid.uuid4())[:8].upper() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Blockchain feature: calculate integrity hash for the grievance | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Performance Boost: Use thread-safe cache to eliminate DB query for last hash | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| prev_hash = grievance_last_hash_cache.get("last_hash") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: The read-compute-write cycle on Prompt for AI agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
| if prev_hash is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Cache miss: Fetch only the last hash from DB | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| prev_grievance = db.query(Grievance.integrity_hash).order_by(Grievance.id.desc()).first() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| prev_hash = prev_grievance[0] if prev_grievance and prev_grievance[0] else "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| grievance_last_hash_cache.set(data=prev_hash, key="last_hash") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # SHA-256 chaining for grievance integrity | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| hash_content = f"{unique_id}|{grievance_data.get('category', 'general')}|{severity.value}|{prev_hash}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| integrity_hash = hashlib.sha256(hash_content.encode()).hexdigest() | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Update cache for next grievance | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| grievance_last_hash_cache.set(data=integrity_hash, key="last_hash") | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+89
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Race condition in concurrent grievance hash computation. The get-compute-set pattern between lines 91-103 is not atomic. Under concurrent grievance creation:
This breaks the blockchain chain integrity since two records will share the same predecessor. 🔧 Suggested fix: Use a dedicated lock for hash computation+import threading
+
+# Module-level lock for grievance hash computation
+_grievance_hash_lock = threading.Lock()
+
class GrievanceService:
# ...
def create_grievance(self, grievance_data: Dict[str, Any], db: Session = None) -> Optional[Grievance]:
# ... earlier code ...
- # Blockchain feature: calculate integrity hash for the grievance
- prev_hash = grievance_last_hash_cache.get("last_hash")
- if prev_hash is None:
- prev_grievance = db.query(Grievance.integrity_hash).order_by(Grievance.id.desc()).first()
- prev_hash = prev_grievance[0] if prev_grievance and prev_grievance[0] else ""
- grievance_last_hash_cache.set(data=prev_hash, key="last_hash")
-
- hash_content = f"{unique_id}|{grievance_data.get('category', 'general')}|{severity.value}|{prev_hash}"
- integrity_hash = hashlib.sha256(hash_content.encode()).hexdigest()
-
- grievance_last_hash_cache.set(data=integrity_hash, key="last_hash")
+ # Blockchain feature: calculate integrity hash with lock to prevent race conditions
+ with _grievance_hash_lock:
+ prev_hash = grievance_last_hash_cache.get("last_hash")
+ if prev_hash is None:
+ prev_grievance = db.query(Grievance.integrity_hash).order_by(Grievance.id.desc()).first()
+ prev_hash = prev_grievance[0] if prev_grievance and prev_grievance[0] else ""
+
+ hash_content = f"{unique_id}|{grievance_data.get('category', 'general')}|{severity.value}|{prev_hash}"
+ integrity_hash = hashlib.sha256(hash_content.encode()).hexdigest()
+ grievance_last_hash_cache.set(data=integrity_hash, key="last_hash")📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+102
to
+103
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cache-DB inconsistency risk on transaction rollback. Line 103 updates the cache with the new Consider updating the cache only after successful commit: 🔧 Suggested fix # Compute hash but defer cache update
hash_content = f"{unique_id}|{grievance_data.get('category', 'general')}|{severity.value}|{prev_hash}"
integrity_hash = hashlib.sha256(hash_content.encode()).hexdigest()
-
- # Update cache for next grievance
- grievance_last_hash_cache.set(data=integrity_hash, key="last_hash")
# ... create grievance object ...
db.add(grievance)
db.commit()
db.refresh(grievance)
+ # Update cache only after successful commit
+ grievance_last_hash_cache.set(data=integrity_hash, key="last_hash")
+
return grievanceAlso applies to: 132-134 🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: Cache is updated with the new Prompt for AI agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Extract location data | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| location_data = grievance_data.get('location', {}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
RohanExploit marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| latitude = location_data.get('latitude') if isinstance(location_data, dict) else None | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -106,7 +124,9 @@ def create_grievance(self, grievance_data: Dict[str, Any], db: Session = None) - | |||||||||||||||||||||||||||||||||||||||||||||||||||
| assigned_authority=assigned_authority, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| sla_deadline=sla_deadline, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| status=GrievanceStatus.OPEN, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue_id=grievance_data.get('issue_id') | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| issue_id=grievance_data.get('issue_id'), | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| integrity_hash=integrity_hash, | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| previous_integrity_hash=prev_hash | ||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
| db.add(grievance) | ||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider max_size for grievance_last_hash_cache with concurrent requests.
grievance_last_hash_cachehasmax_size=1, which is appropriate for a single "last_hash" key. However, inbackend/grievance_service.py, there's a race condition window betweenget("last_hash")andset("last_hash")when multiple concurrent grievance creations occur. The cache itself is thread-safe, but the get-compute-set pattern is not atomic.This could result in two concurrent grievances computing their hash using the same
prev_hash, leading to incorrect chain linkage. Consider using a lock around the entire hash computation block inGrievanceService.create_grievance().🤖 Prompt for AI Agents