Skip to content

Conversation

@abhishek10004
Copy link
Collaborator

Description

  1. Destroy MRDinstance & remove it from cache when inode is destroyed.
  2. Handled an edge case, when MRDInstance was re-added to the cache before eviction, in DecrementRefCount flow.
  3. Refactored the code.
  4. Added UTs

Link to the issue in case of a bug fix.

b/474070647

Testing details

  1. Manual - NA
  2. Unit tests - Done
  3. Integration tests - NA

Any backward incompatible change? If so, please explain.

@abhishek10004
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 important improvements to the lifecycle management of MrdInstance. By ensuring that the MrdInstance is destroyed and removed from the cache when its owning inode is destroyed, it effectively prevents potential resource leaks. The refactoring of the cache eviction logic into the MrdInstance itself is a great change that improves encapsulation and clarity. I was particularly impressed with the handling of the race condition where an instance could be re-added to the cache before its eviction is processed. The new unit tests are comprehensive and thoroughly cover the new logic and edge cases. The changes are well-implemented and significantly enhance the robustness of the code. Great work!

@codecov
Copy link

codecov bot commented Jan 9, 2026

Codecov Report

❌ Patch coverage is 89.65517% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.10%. Comparing base (fe4a578) to head (6cd4d67).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
internal/gcsx/mrd_instance.go 87.50% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4262      +/-   ##
==========================================
+ Coverage   83.06%   83.10%   +0.03%     
==========================================
  Files         159      159              
  Lines       19235    19249      +14     
==========================================
+ Hits        15977    15996      +19     
+ Misses       2666     2663       -3     
+ Partials      592      590       -2     
Flag Coverage Δ
unittests 83.10% <89.65%> (+0.03%) ⬆️

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.

@abhishek10004 abhishek10004 marked this pull request as ready for review January 9, 2026 20:36
@abhishek10004 abhishek10004 requested a review from a team as a code owner January 9, 2026 20:36
@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 9, 2026
@github-actions
Copy link

Hi @raj-prince, @vadlakondaswetha, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you!

1 similar comment
@github-actions
Copy link

Hi @raj-prince, @vadlakondaswetha, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you!

raj-prince
raj-prince previously approved these changes Jan 13, 2026
Copy link
Collaborator

@raj-prince raj-prince left a comment

Choose a reason for hiding this comment

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

LGTM.

raj-prince
raj-prince previously approved these changes Jan 13, 2026
@abhishek10004 abhishek10004 enabled auto-merge (squash) January 14, 2026 05:31
@abhishek10004 abhishek10004 merged commit 2732b25 into master Jan 14, 2026
15 checks passed
@abhishek10004 abhishek10004 deleted the abhishek/inode_destroy branch January 14, 2026 05:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

remind-reviewers Auto remind reviewers in attention set for review post 24hrs of inactivity on PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants