Conversation
a thread-safe caching mechanism at the connector level. Changes: - Add cache fields to FileConnector for loaded data and built caches - Implement getCachedData() method with double-check locking pattern - Update fileSyncer to reference connector instead of file path - Modify List(), Entitlements(), and Grants() to use cached data Performance Impact: - File loaded once per connector instance instead of per-method call - Eliminates ~9x redundant file reads during typical sync operations - Expected 8-18x improvement for large files (10k+ rows)
1. Added child types index field in pkg/connector/models.go:
- Added cachedChildTypes map[string]map[string]struct{} to FileConnector
- Maps parent resource ID → set of child resource type IDs
2. Created buildChildTypesIndex() function in pkg/connector/cache.go:
- Builds the index once during cache creation
- Iterates through all resources and maps children to their parents
- Returns a map for O(1) lookups
3. Updated getCachedData() in pkg/connector/connector.go:
- Calls buildChildTypesIndex() and stores result in cache
- Returns the child types index along with other caches
- Signature updated to return 6 values instead of 5
4. Optimized List() method in pkg/connector/syncers.go:
- Replaced O(n) loop scanning all resources with O(1) index lookup
- Changed from iterating loadedData.Resources to simple map lookup: childTypesIndex[resource.Id.Resource]
- Removed unused strings import
Performance Impact:
Before:
- For each paginated resource (up to 50 per page), scanned ALL resources in the file
- 50 resources × 1,000 total resources = 50,000 comparisons per page
- O(n*m) complexity where n = resources per page, m = total resources
After:
- Direct O(1) map lookup for each resource
- 50 resources × 1 lookup = 50 lookups per page
- ~1000x improvement for large datasets
1. Added sorted index fields in pkg/connector/models.go: - cachedSortedResourcesByType map[string][]*v2.Resource - Pre-sorted resources grouped by type - cachedSortedEntitlementsByRes map[string][]*v2.Entitlement - Pre-sorted entitlements grouped by resource 2. Created sorting functions in pkg/connector/cache.go: - buildSortedResourcesByType() - Groups resources by type and sorts each group once - buildSortedEntitlementsByResource() - Groups entitlements by resource and sorts each group once 3. Updated getCachedData() in pkg/connector/connector.go: - Calls the sorting functions during cache building - Stores sorted indexes in the connector cache 4. Optimized List() method in pkg/connector/syncers.go: - Retrieves pre-sorted resources from cached index - Only filters by parent if needed (no type filtering or sorting) - Eliminated O(n log n) sort on every page request 5. Optimized Entitlements() method in pkg/connector/syncers.go: - Retrieves pre-sorted entitlements from cached index - No filtering or sorting needed - direct lookup - Eliminated O(n log n) sort on every page request Note: Grants() method still sorts on each request because grants are filtered dynamically by context (principal or target). Pre-building a sorted index for grants would be complex and may not provide significant benefit given their filtering requirements. Performance Impact: Before: - List(): Filter all resources + sort all matching resources on every page (20× for 1000 resources with page size 50) - Entitlements(): Filter all entitlements + sort all matching entitlements on every page - Total: ~20 × O(n log n) operations per sync After: - List(): O(1) map lookup + O(m) parent filter where m = resources of this type (no sorting) - Entitlements(): O(1) map lookup (no filtering or sorting) - Sorting happens once during cache building: O(n log n) total Estimated speedup: 10-20x faster for pagination scenarios with multiple pages.
- defaultPageSize = 200 - Centralized constant replacing hardcoded values - Documented reasoning: all data is in-memory and already pre-sorted/filtered 2. Updated all three syncer methods: - List(): Changed pageSize := 50 to pageSize := defaultPageSize - Entitlements(): Changed pageSize := 50 to pageSize := defaultPageSize - Grants(): Changed pageSize := 50 to pageSize := defaultPageSize Performance Impact: For 1,000 resources: - Before (page size 50): 20 pagination calls needed - After (page size 200): 5 pagination calls needed - Reduction: 4x fewer API round trips Combined with our previous optimizations: - Each pagination call is now 10-20x faster (no redundant file reads, no sorting) - 4x fewer pagination calls (larger page size) - Total estimated speedup: 40-80x for pagination scenarios Why Page Size 200? - All data is in-memory (no I/O concerns) - Pre-sorted and pre-filtered (minimal processing per request) - No external API rate limits - Balances memory usage with performance - Easy to adjust via the constant if needed
WalkthroughAdds a thread-safe, lazy-initialized in-memory cache to FileConnector (getCachedData, clearCache, cache fields and helper builders); refactors fileSyncer and syncer handlers to use prebuilt, sorted caches and standardized pagination instead of per-call file reads and sorts. Changes
Sequence Diagram(s)sequenceDiagram
participant Sync as Sync Operation (List/Entitlements/Grants)
participant FC as FileConnector
participant Cache as In-Memory Cache
participant Loader as File Loader / Builders
Sync->>FC: getCachedData(ctx)
alt Cache hit
FC->>Cache: read cachedData & indexes (read lock)
FC-->>Sync: return cached structures
else Cache miss
FC->>FC: acquire write lock
FC->>Loader: LoadFileData()
Loader-->>FC: LoadedData
FC->>Loader: buildChildTypesIndex(LoadedData)
Loader-->>FC: childTypesIndex
FC->>Loader: buildSortedResourcesByType(LoadedData)
Loader-->>FC: sortedResources
FC->>Loader: buildSortedEntitlementsByResource(LoadedData)
Loader-->>FC: sortedEntitlements
FC->>Cache: store caches & indexes
FC->>FC: release write lock
FC-->>Sync: return cached structures
end
Sync->>Cache: filter & paginate using cached indexes
Sync-->>Client: return results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)pkg/connector/connector.go (2)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting Comment |
pkg/connector/connector.go
Outdated
| } | ||
|
|
||
| // clearCache invalidates all cached data, forcing a fresh load on the next getCachedData call. | ||
| // This is called at the start of each sync operation to ensure fresh data between syncs. |
There was a problem hiding this comment.
the new session cache does this automatically, see
session.SetManyJSON(ctx, ss, newProjects, projectsNamespace)
in this PR. Each sync is a session.
There was a problem hiding this comment.
I don't see you having actually made the changes required to use the session cache, it looks like you're still storing things in memory. I'm not sure if it is available for on prem. I would hope so.
3 participants
Description
Useful links:
Summary by CodeRabbit
Performance
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.