Skip to content

Implement bypass_cache flag for Tier1 and other fixes#137

Merged
tokebe merged 12 commits intomainfrom
t1_cache_bypass
Mar 6, 2026
Merged

Implement bypass_cache flag for Tier1 and other fixes#137
tokebe merged 12 commits intomainfrom
t1_cache_bypass

Conversation

@shuchenliu
Copy link
Contributor

@shuchenliu shuchenliu commented Mar 3, 2026

This PR solves tier1 part of #45 and added other fixes

  1. Fixed flaky test in metadata_retrieval
    Resolved an issue where stale data in the Redis instance could cause test failures when new indices were deployed.

  2. Enabled bypass_cache for get_subclass_mapping
    Added support for the bypass_cache flag in the driver.get_subclass_mapping method to address similar cache-related issues.

  3. Implemented query-level bypass_cache support
    Added support for the TRAPI-level bypass_cache flag, allowing clients to purge Elasticsearch’s query, request, and fielddata caches when bypass_cache: true is specified in the original TRAPI query, by updatingdriver.run_query method to accept an additional bypass_cache keyword parameter.

@sentry
Copy link

sentry bot commented Mar 3, 2026

Codecov Report

❌ Patch coverage is 52.00000% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../retriever/data_tiers/tier_1/elasticsearch/meta.py 0.00% 7 Missing ⚠️
...etriever/data_tiers/tier_1/elasticsearch/driver.py 50.00% 2 Missing and 1 partial ⚠️
..._tiers/tier_1/elasticsearch/aggregating_querier.py 83.33% 1 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ
..._tiers/tier_1/elasticsearch/aggregating_querier.py 32.83% <83.33%> (+9.62%) ⬆️
...etriever/data_tiers/tier_1/elasticsearch/driver.py 50.00% <50.00%> (+4.83%) ⬆️
.../retriever/data_tiers/tier_1/elasticsearch/meta.py 22.64% <0.00%> (-0.29%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds cache-bypass capabilities to Tier 1 Elasticsearch metadata/subclass-mapping retrieval and query execution, primarily to mitigate stale-cache issues and related test flakiness (Issue #45).

Changes:

  • Add bypass_cache support to Tier 1 ES query execution by clearing ES index caches before running queries when requested.
  • Add bypass_cache support to ubergraph subclass mapping retrieval (skip Redis metadata cache when requested).
  • Update Tier 1 ES driver tests to use cache bypass and add a new cache-bypass test.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/retriever/data_tiers/tier_1/elasticsearch/driver.py Adds bypass_cache handling for query execution and passes bypass flag through subclass-mapping retrieval.
src/retriever/data_tiers/tier_1/elasticsearch/meta.py Adds bypass_cache option to ubergraph info retrieval to skip cached values when requested.
tests/data_tiers/tier_1/elasticsearch_tests/test_tier1_driver.py Updates integration tests to bypass cache for metadata/subclass mapping and adds a cache-bypass query test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Contributor

Copilot AI commented Mar 3, 2026

@shuchenliu I've opened a new pull request, #138, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot AI and others added 2 commits March 2, 2026 20:49
…pendency (#138)

test: mock clear_cache and run_single_query in test_cache_bypass

Co-authored-by: shuchenliu <5944967+shuchenliu@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: shuchenliu <5944967+shuchenliu@users.noreply.github.com>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@tokebe
Copy link
Contributor

tokebe commented Mar 4, 2026

@shuchenliu Just checking, this now does a timestamp check instead of purging the cache, and is ready for review?

@shuchenliu
Copy link
Contributor Author

@shuchenliu Just checking, this now does a timestamp check instead of purging the cache, and is ready for review?

Yes, we no longer purge the cache. A timestamp check payload is no added to every query

@tokebe tokebe merged commit ba0463a into main Mar 6, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants