-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Add miss_count and removed_in_bytes fields in /_nodes/stats/file_cache API and fix used_in_bytes field collision #19656
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?
Conversation
|
❌ Gradle check result for 5fc979c: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 658c9a5: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
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.
Changes looks good to me.
server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheStats.java
Outdated
Show resolved
Hide resolved
658c9a5 to
2c1bef7
Compare
|
❌ Gradle check result for 2c1bef7: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
Gradle Check failed due to Flaky Tests : #17486 |
2c1bef7 to
e095205
Compare
|
❌ Gradle check result for e095205: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
server/src/main/java/org/opensearch/index/store/remote/filecache/FileCacheStats.java
Outdated
Show resolved
Hide resolved
|
❌ Gradle check result for e095205: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
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.
Change looks good to me now.
|
❌ Gradle check result for e095205: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
…s fields Add cache miss tracking and explicit removal tracking to file cache statistics: Add miss_count field to all nested statistics objects for hit ratio analysis Add removed_in_bytes field to distinguish manual removals from automatic evictions Update FileCacheStats with getRemoved() method and enhanced JSON serialization Update AggregateFileCacheStats with aggregate JSON serialization for new fields Update FileCache fileCacheStats() method to pass removal data Add comprehensive test coverage including MissCountValidationTest Update all FileCacheStats constructor calls to include removed parameter Enables hit ratio calculation, cache pressure monitoring, and performance debugging across different file types in the cache hierarchy. Signed-off-by: tanishq ranjan <[email protected]>
Signed-off-by: tanishq ranjan <[email protected]>
Signed-off-by: tanishq ranjan <[email protected]>
Signed-off-by: tanishq ranjan <[email protected]>
e095205 to
40c08c9
Compare
|
❌ Gradle check result for 40c08c9: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 40c08c9: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
All FileCacheStats constructor signature issues have been resolved: - Fixed 24 constructor calls across 4 test files - Added version compatibility for V_3_4_0 - All gradle build and test commands pass locally
|
❌ Gradle check result for 42c9c8f: null Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
|
❌ Gradle check result for 42c9c8f: FAILURE Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change? |
Summary
This CR enhances the OpenSearch
/_nodes/stats/file_cacheAPI by adding comprehensive cache monitoring capabilities and fixes a critical JSON field-name collision that caused nestedused_in_bytesto be reported as0.PINNED_IN_BYTESand stop writing pinned usage toUSED_IN_BYTES(removes field-name collision so nestedused_in_bytesnow report actual usage)API Response Enhancement
Before (Incomplete Visibility):
{ "_nodes": {"total": 1, "successful": 1, "failed": 0}, "cluster_name": "REDACTED:REDACTED-REDACTED", "nodes": { "_-REDACTED": { "aggregate_file_cache": { "active_in_bytes": 0, "total_in_bytes": 368353338982, "used_in_bytes": 0, "pinned_in_bytes": 0, "evictions_in_bytes": 0, "hit_count": 1008237, "miss_count": 904846, // Only at ROOT level "over_all_stats": { "active_in_bytes": 0, "used_in_bytes": 0, "evictions_in_bytes": 0, "hit_count": 1008237 // ❌ MISSING: miss_count // ❌ MISSING: removed_in_bytes }, "full_file_stats": { "active_in_bytes": 0, "used_in_bytes": 0, "evictions_in_bytes": 0, "hit_count": 69 // ❌ MISSING: miss_count // ❌ MISSING: removed_in_bytes }, "block_file_stats": { "active_in_bytes": 0, "used_in_bytes": 0, "evictions_in_bytes": 0, "hit_count": 1008168 // ❌ MISSING: miss_count // ❌ MISSING: removed_in_bytes }, "pinned_file_stats": { "active_in_bytes": 0, "used_in_bytes": 0, "evictions_in_bytes": 0, "hit_count": 0 // ❌ MISSING: miss_count // ❌ MISSING: removed_in_bytes } } } } }After:
{ "_nodes": {"total": 1, "successful": 1, "failed": 0}, "cluster_name": "REDACTED:REDACTED-REDACTED", "nodes": { "REDACTED": { "aggregate_file_cache": { "active_in_bytes": 0, "active": "0b", "total_in_bytes": 368353338982, "total": "343.2gb", "used_in_bytes": 0, "used": "0b", "pinned_in_bytes": 0, "pinned": "0b", "evictions_in_bytes": 0, "evictions": "0b", "removed_in_bytes": 67890, // NEW: Total explicit removals "removed": "66.3kb", // NEW: Human readable "hit_count": 1008237, "miss_count": 904846, "over_all_stats": { "active_in_bytes": 0, "active": "0b", "used_in_bytes": 382533541, "used": "364.9mb", "evictions_in_bytes": 0, "evictions": "0b", "removed_in_bytes": 67890, // NEW: Overall removals "removed": "66.3kb", "active_percent": 0, "hit_count": 1008237, "miss_count": 904846 // NEW: Overall miss count }, "full_file_stats": { "active_in_bytes": 0, "active": "0b", "used_in_bytes": 41445300, "used": "39.5mb", "evictions_in_bytes": 0, "evictions": "0b", "removed_in_bytes": 22212, // NEW "removed": "21.7kb", "active_percent": 0, "hit_count": 69, "miss_count": 0 // NEW }, "block_file_stats": { "active_in_bytes": 0, "active": "0b", "used_in_bytes": 341088241, "used": "325.2mb", "evictions_in_bytes": 0, "evictions": "0b", "removed_in_bytes": 45678, // NEW "removed": "44.6kb", "active_percent": 0, "hit_count": 1008168, "miss_count": 904846 // NEW }, "pinned_file_stats": { "active_in_bytes": 0, "active": "0b", "used_in_bytes": 0, "used": "0b", "evictions_in_bytes": 0, "evictions": "0b", "removed_in_bytes": 0, // NEW "removed": "0b", "active_percent": 0, "hit_count": 0, "miss_count": 0 // NEW } } } } }