fix: overwrite aggs cache instead of delete and rewrite cache #9390

Loaki07 · 2025-11-28T13:57:24Z

How I run my query

nov 26 10:30 - 11:30 - interval 30 mins
nov 26 10:30 - 16:30 - interval 60 mins
nov 26 10:30 - 16:30 - interval 60 mins -- refresh cache

Cache dir

on first run

has 2 caches 30 mins

on 2nd run

will havve mixed 30mins cache and 60 mins cache

TODO:

Problem 1:
on step 3
the interval for that duration is 60 mins
but the discovery of cache will also include the 30 mins cache @mod.rs discover_cache_for_query
the overwrite will over write the 60 mins cache only. How to address this problem?
Problem 2:
fix: add rbac control only user with permussion to edit stream can refresh cache

Proposed Solution

On refresh_cache run streaming aggs without using cache
at streaming/execution.rs , at the end of steaming aggs delete all the cache files for the given duration when there are miss matched cache interval files
then remove the streaming_id from the GLOBAL_CACHE
eg: nov 26 10:30 - 16:30 - interval 60 mins -- refresh cache
Cache consists of 30mins or 4hr interval files all of them will be deleted
60 mins cache interval files will be overwritten or created a new.
This way on the next run there will be no query results mismatch when the query runs again with the newly updated cache, especially delayed data for the same duration.
add rbac control to only allow users with edit permissions to stream to use this feature

github-actions · 2025-11-28T13:58:05Z

Failed to generate code suggestions for PR

greptile-apps · 2025-11-28T14:00:34Z

Greptile Overview

Greptile Summary

This PR adds a use_aggs_cache parameter to the search_partition function to control whether aggregation cache is used, implementing the ability to skip cache when req.use_cache=false.

Key Changes:

Added use_aggs_cache parameter to search_partition() function signature
Modified cache discovery logic to skip cache lookup when use_aggs_cache=false
Updated streaming/execution.rs to correctly pass req.use_cache value

Critical Issue:

The HTTP and gRPC handlers hardcode use_aggs_cache=true instead of respecting the user's use_cache preference. This happens because SearchPartitionRequest struct doesn't have a use_cache field to carry this information from the client request. As a result, the PR doesn't fully achieve its goal - users setting use_cache=false will still have aggregation cache enabled when calling the _search_partition endpoint directly.
Only the streaming execution path (used by the main search API) correctly honors req.use_cache

Confidence Score: 2/5

This PR has a critical logic flaw that prevents it from fully achieving its stated goal
The implementation is incomplete - HTTP and gRPC handlers hardcode use_aggs_cache=true, which means users cannot disable aggregation cache through the _search_partition endpoint even when setting use_cache=false. The SearchPartitionRequest struct needs a use_cache field to properly propagate the user's preference.
Pay close attention to src/handler/grpc/request/search/mod.rs and src/handler/http/request/search/mod.rs - both need to respect the use_cache preference instead of hardcoding true

Important Files Changed

File Analysis

Filename	Score	Overview
src/handler/grpc/request/search/mod.rs	2/5	Hardcodes `use_aggs_cache=true` instead of respecting user's cache preference from request
src/handler/http/request/search/mod.rs	2/5	Hardcodes `use_aggs_cache=true` instead of respecting user's cache preference from request
src/service/search/mod.rs	4/5	Added `use_aggs_cache` parameter and correctly skips cache discovery when disabled

Sequence Diagram

sequenceDiagram
    participant Client
    participant HTTP/gRPC Handler
    participant SearchService
    participant CacheDiscovery
    participant QueryExecution

    Client->>HTTP/gRPC Handler: Request with use_cache=false
    Note over HTTP/gRPC Handler: SearchPartitionRequest missing use_cache field
    HTTP/gRPC Handler->>SearchService: search_partition(..., use_aggs_cache=true)
    Note over HTTP/gRPC Handler: ⚠️ Hardcoded true ignores user preference
    
    alt use_aggs_cache=false
        SearchService->>SearchService: Create empty CacheDiscoveryResult
        Note over SearchService: Skip cache lookup entirely
    else use_aggs_cache=true
        SearchService->>CacheDiscovery: discover_cache_for_query()
        CacheDiscovery-->>SearchService: Return cached ranges
    end
    
    SearchService->>QueryExecution: Execute query with/without cache
    QueryExecution-->>SearchService: Results
    SearchService-->>HTTP/gRPC Handler: Response
    HTTP/gRPC Handler-->>Client: Search results
    
    Note over Client,QueryExecution: ✅ streaming/execution.rs correctly uses req.use_cache

greptile-apps

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

src/handler/grpc/request/search/mod.rs

src/handler/http/request/search/mod.rs

overwritten

`refresh cache`

How I run my query 1. nov 26 10:30 - 11:30 - interval 30 mins 2. nov 26 10:30 - 16:30 - interval 60 mins 3. nov 26 10:30 - 16:30 - interval 60 mins -- refresh cache Cache dir 1. on first run - has 2 caches 30 mins 2. on 2nd run - will havve mixed 30mins cache and 60 mins cache TODO: - [x] Problem 1: on step 3 - the interval for that duration is 60 mins - but the discovery of cache will also include the 30 mins cache [@mod.rs](file:///Users/lin/o2/openobserve/src/service/search/mod.rs) `discover_cache_for_query` - the overwrite will over write the 60 mins cache only. How to address this problem? - [x] Problem 2: - fix: add rbac control only user with permussion to edit stream can `refresh cache` Proposed Solution - On `refresh_cache` run streaming aggs without using cache - at `streaming/execution.rs` , at the end of steaming aggs delete all the cache files for the given duration when there are miss matched cache interval files - then remove the `streaming_id` from the `GLOBAL_CACHE` - eg: nov 26 10:30 - 16:30 - interval 60 mins -- refresh cache - Cache consists of 30mins or 4hr interval files all of them will be deleted - 60 mins cache interval files will be overwritten or created a new. - This way on the next run there will be no query results mismatch when the query runs again with the newly updated cache, especially delayed data for the same duration. - add rbac control to only allow users with edit permissions to stream to use this feature

…#9396) ref: - #9390 - openobserve/o2-enterprise#974

fix: skip aggs cache when req.use_cache=false

e311550

github-actions bot added the ☢️ Bug Something isn't working label Nov 28, 2025

greptile-apps bot reviewed Nov 28, 2025

View reviewed changes

src/handler/grpc/request/search/mod.rs Outdated Show resolved Hide resolved

src/handler/http/request/search/mod.rs Outdated Show resolved Hide resolved

fix: owerwrite aggs cache instead of delete and replase cache

67050df

Loaki07 changed the title ~~fix: skip aggs cache when req.use_cache=false~~ fix: overwrite aggs cache instead of delete and replace cache Nov 28, 2025

Loaki07 requested review from haohuaijin, hengfeiyang and oasisk November 28, 2025 15:47

Merge branch 'main' into fix/streaming_aggs_refresh_cache

f53e7a7

Loaki07 changed the title ~~fix: overwrite aggs cache instead of delete and replace cache~~ fix: overwrite aggs cache instead of delete and rewrite cache Nov 28, 2025

Loaki07 added 2 commits November 28, 2025 21:37

chore: clippy

c582392

chore: address comments

1e9d5f9

oasisk approved these changes Nov 28, 2025

View reviewed changes

Loaki07 and others added 3 commits November 28, 2025 22:24

Merge branch 'main' into fix/streaming_aggs_refresh_cache

3a5b929

fix: result mismatch due to mismatched interval files not being

9b99af3

overwritten

fix: add rbac control only user with permussion to edit stream can

ae2df00

`refresh cache`

Loaki07 merged commit baa7520 into main Nov 29, 2025
37 checks passed

Loaki07 deleted the fix/streaming_aggs_refresh_cache branch November 29, 2025 07:19

This was referenced Nov 29, 2025

fix: overwrite aggs cache instead of delete and rewrite cache (#9390) #9396

Merged

An error occurs when clicking the “Refresh cache & run query” button on the Logs page. #9387

Closed

Loaki07 added a commit that referenced this pull request Nov 29, 2025

fix: overwrite aggs cache instead of delete and rewrite cache (#9390) (…

70454dc

…#9396) ref: - #9390 - openobserve/o2-enterprise#974

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: overwrite aggs cache instead of delete and rewrite cache #9390

fix: overwrite aggs cache instead of delete and rewrite cache #9390

Uh oh!

Loaki07 commented Nov 28, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Nov 28, 2025

Uh oh!

greptile-apps bot commented Nov 28, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fix: overwrite aggs cache instead of delete and rewrite cache #9390

fix: overwrite aggs cache instead of delete and rewrite cache #9390

Uh oh!

Conversation

Loaki07 commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 28, 2025

Uh oh!

greptile-apps bot commented Nov 28, 2025

Greptile Overview

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Loaki07 commented Nov 28, 2025 •

edited

Loading