Skip to content

Conversation

@Tulsishah
Copy link
Collaborator

@Tulsishah Tulsishah commented Jan 5, 2026

Description

In GCSFuse, an "implicit directory" is a folder that exists as a prefix to other objects but does not have a corresponding 0-byte object in GCS. Historically, these were tracked in a separate typeCache. This PR migrates that responsibility to the statCache by inferring directory existence from listing results.

Link to the issue in case of a bug fix.

b/473410044

Testing details

  1. Manual - NA
  2. Unit tests - Added
  3. Integration tests - Automated

Any backward incompatible change? If so, please explain.

@Tulsishah Tulsishah added execute-integration-tests Run only integration tests execute-integration-tests-on-zb To run E2E tests on zonal bucket. labels Jan 5, 2026
@codecov
Copy link

codecov bot commented Jan 5, 2026

Codecov Report

❌ Patch coverage is 88.88889% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.12%. Comparing base (54895d1) to head (8e9ccb4).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
internal/storage/caching/fast_stat_bucket.go 88.88% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4237      +/-   ##
==========================================
+ Coverage   83.08%   83.12%   +0.04%     
==========================================
  Files         154      154              
  Lines       18913    18973      +60     
==========================================
+ Hits        15714    15772      +58     
  Misses       2625     2625              
- Partials      574      576       +2     
Flag Coverage Δ
unittests 83.12% <88.88%> (+0.04%) ⬆️

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.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Tulsishah Tulsishah force-pushed the fast_stat_bucket_cache_implicit_dir branch from a4479cb to 21aa3d7 Compare January 5, 2026 08:49
@Tulsishah
Copy link
Collaborator Author

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces new logic to handle implicit directories when caching GCS listings, controlled by a new IsTypeCacheDeprecated flag. The changes include a new insertListing function in fastStatBucket and a corresponding test suite. My review has identified a bug in the implicit directory detection logic, where directories containing only subdirectories are not correctly cached. I've also found an issue in the corresponding test case that fails to check for this, and a potential for test flakiness due to using map iteration for ordered mock expectations. I've provided suggestions to fix these issues.

@Tulsishah Tulsishah force-pushed the fast_stat_bucket_cache_implicit_dir branch from f2f962e to bc1817e Compare January 5, 2026 09:16
@Tulsishah Tulsishah force-pushed the fast_stat_bucket_cache_implicit_dir branch from 2514414 to 7d450c4 Compare January 5, 2026 09:20
@Tulsishah Tulsishah marked this pull request as ready for review January 6, 2026 04:31
@Tulsishah Tulsishah requested a review from a team as a code owner January 6, 2026 04:31
@github-actions github-actions bot added the remind-reviewers Auto remind reviewers in attention set for review post 24hrs of inactivity on PR. label Jan 6, 2026
@Tulsishah Tulsishah requested review from vadlakondaswetha and removed request for abhishek10004 January 6, 2026 04:31
@Tulsishah Tulsishah removed execute-integration-tests Run only integration tests execute-integration-tests-on-zb To run E2E tests on zonal bucket. labels Jan 6, 2026
@Tulsishah Tulsishah added type-cache-deprecation and removed remind-reviewers Auto remind reviewers in attention set for review post 24hrs of inactivity on PR. labels Jan 6, 2026
@Tulsishah Tulsishah enabled auto-merge (squash) January 6, 2026 09:32
@Tulsishah Tulsishah merged commit 60911bc into master Jan 6, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants