Skip to content
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

Negative cache TTL Fix #2837

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Negative cache TTL Fix #2837

wants to merge 16 commits into from

Conversation

ankitaluthra1
Copy link
Collaborator

@ankitaluthra1 ankitaluthra1 commented Dec 24, 2024

Description

  1. This PR adds a new hidden flag for negative cache TTL.
  2. The value defaults to 5 secs.
  3. The changes include unit tests and are manually verified on GKE as well.
  4. Integration tests will be added in subsequent PR.

Link to the issue in case of a bug fix.
NA

Testing details
Manual - Yes
Unit tests - Yes
Integration tests - NA

@ankitaluthra1 ankitaluthra1 added execute-perf-test Execute performance test in PR execute-integration-tests Run only integration tests labels Dec 24, 2024
@ankitaluthra1 ankitaluthra1 marked this pull request as ready for review December 24, 2024 09:42
@ankitaluthra1 ankitaluthra1 requested a review from a team as a code owner December 24, 2024 09:42
@ankitaluthra1 ankitaluthra1 requested review from raj-prince, charith87 and Tulsishah and removed request for raj-prince December 24, 2024 09:42
@kislaykishore kislaykishore requested review from raj-prince, a team, gargnitingoogle and tritone and removed request for charith87, Tulsishah, a team and gargnitingoogle December 24, 2024 09:42
@ankitaluthra1 ankitaluthra1 requested review from charith87 and removed request for raj-prince, tritone and a team December 24, 2024 11:31
@kislaykishore kislaykishore requested review from tritone and a team December 24, 2024 11:32
Copy link

codecov bot commented Dec 24, 2024

Codecov Report

Attention: Patch coverage is 45.71429% with 19 lines in your changes missing coverage. Please review.

Project coverage is 75.77%. Comparing base (ff4417a) to head (8d63c9b).
Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
cfg/config.go 25.00% 4 Missing and 2 partials ⚠️
cfg/validate.go 14.28% 4 Missing and 2 partials ⚠️
cfg/rationalize.go 40.00% 2 Missing and 1 partial ⚠️
internal/gcsx/bucket_manager.go 0.00% 2 Missing ⚠️
cmd/mount.go 0.00% 1 Missing ⚠️
internal/storage/caching/fast_stat_bucket.go 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2837      +/-   ##
==========================================
- Coverage   75.94%   75.77%   -0.18%     
==========================================
  Files         118      118              
  Lines       16320    16431     +111     
==========================================
+ Hits        12394    12450      +56     
- Misses       3409     3449      +40     
- Partials      517      532      +15     
Flag Coverage Δ
unittests 75.77% <45.71%> (-0.18%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@@ -66,7 +69,10 @@ type fastStatBucket struct {
// Constant data
/////////////////////////

ttl time.Duration
// TTL for entries for existing files in the cache.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: files -> files and folders

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
execute-integration-tests Run only integration tests execute-perf-test Execute performance test in PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants