Unified Storage Performance Optimizations

[hussam789]

User description

PR #10

---

PR Type

Enhancement, Bug fix

---

Description

• Move unified storage initialization to startup instead of lazy initialization

• Fix trace context propagation by capturing span context correctly

• Optimize index building with fine-grained cache locking

• Replace deprecated logger with structured logging

• Skip postgres integration test due to Drone environment issues

---

Diagram Walkthrough

flowchart LR
  A["Server Creation"] -->|"eager init"| B["Search Index Init"]
  B -->|"fine-grained locking"| C["Index Building"]
  D["Trace Context"] -->|"capture ctx"| E["Span Propagation"]
  F["Logging"] -->|"replace logger"| G["Structured Logs"]

Loading

File Walkthrough

	Relevant files
Tests	module_server_test.go Skip postgres integration test in Drone                                    pkg/server/module_server_test.go Added skip condition for postgres database type in integration test Prevents test failures in Drone CI environment while working locally +4/-0
Bug fix	search.go Fix trace context and improve logging                                        pkg/storage/unified/resource/search.go Fixed trace context propagation by capturing span context in init() and build() methods Replaced deprecated logger.Warn() with s.log.Warn() for consistent structured logging Added info log for search index initialization with duration and document count metrics Removed unused import of grafana-cli/logger +4/-4      backend.go Fix trace context propagation                                                        pkg/storage/unified/sql/backend.go Fixed trace context propagation in GetResourceStats() by capturing span context +1/-1
Enhancement	server.go Eager initialization and remove lazy init calls                    pkg/storage/unified/resource/server.go Moved Init() call from lazy initialization in each method to eager initialization in NewResourceServer() Reordered initialization sequence to initialize search index before starting watcher Removed redundant Init() calls from Create, Update, Delete, Read, List, Watch, Search, History, Origin, IsHealthy, PutBlob, and GetBlob methods +11/-46  bleve.go Optimize cache locking in index building                                  pkg/storage/unified/search/bleve.go Moved cache lock acquisition to after index building completes for fine-grained locking Removed lock from beginning of BuildIndex() method to reduce contention Simplified TODO comment formatting for index version checking +5/-6

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Concurrency race risk Description: The index cache map 'b.cache' is written with only a narrow critical section that omits locking around the preceding Index creation and batch operations, risking concurrent visibility of a partially constructed 'idx' if other goroutines read the map; verify all accesses to 'b.cache' are consistently synchronized. bleve.go [129-140] Referred Code } // Flush the batch err = idx.Flush() if err != nil { return nil, err } b.cacheMu.Lock() b.cache[key] = idx b.cacheMu.Unlock() return idx, nil
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Limited audit logs: New logs added focus on performance and initialization metrics but do not provide user- or action-scoped audit entries for critical actions within the changed code. Referred Code end := time.Now().Unix() s.log.Info("search index initialized", "duration_secs", end-start, "total_docs", s.search.TotalDocs()) if IndexMetrics != nil { IndexMetrics.IndexCreationTime.WithLabelValues().Observe(float64(end - start)) } return nil } // Async event func (s *searchSupport) handleEvent(ctx context.Context, evt *WrittenEvent) { if !slices.Contains([]WatchEvent_Type{WatchEvent_ADDED, WatchEvent_MODIFIED, WatchEvent_DELETED}, evt.Type) { s.log.Info("ignoring watch event", "type", evt.Type) return } nsr := NamespacedResource{ Namespace: evt.Key.Namespace, Group: evt.Key.Group, Resource: evt.Key.Resource, } ... (clipped 46 lines)
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Locking change risk: Moving cache locking to after index build may expose a race where multiple builders create and assign the same cache entry without guarding earlier phases, which could cause inconsistent state. Referred Code } // Flush the batch err = idx.Flush() if err != nil { return nil, err } b.cacheMu.Lock() b.cache[key] = idx b.cacheMu.Unlock() return idx, nil

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

No code suggestions found for the PR.