feat(simple reader): Updating MRDInstance when minObject is updated in inode

[abhishek10004]

Description

Adding changes to ensure minObject is updated in MrdInstance as well when it's updated in inode. Also ensuring that we decrement the refCount from simple reader only when it was incremented from that reader as well.

Link to the issue in case of a bug fix.

b/474070744

Testing details

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

Any backward incompatible change? If so, please explain.

[gemini-code-assist]

Summary of Changes

Hello @abhishek10004, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses an issue where the minObject in MrdInstance was not being updated in sync with the inode, potentially leading to stale data. It also fixes a refcount issue in MrdSimpleReader that could cause incorrect decrementing. The changes ensure data consistency and proper resource management.

Highlights

• MRD Instance Update: Ensures that the minObject within MrdInstance is updated whenever the minObject in the inode is updated, maintaining data consistency.
• Refcount Management: Modifies the MrdSimpleReader to decrement the reference count of mrdInstance only if it was originally incremented by that specific reader, preventing potential double decrements.
• Code Modernization: Replaces calls to updateMRDWrapper with updateMRD.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request makes two important correctness improvements. First, it ensures that when an inode's backing minObject is updated, the MrdInstance is also updated, using an atomic.Pointer for thread-safe access. Second, it fixes a potential issue in MrdSimpleReader by ensuring the reference count is decremented only if it was previously incremented by that reader, using CompareAndSwap. The associated test refactoring is also a nice cleanup.

However, I've identified a potential data race introduced by the new call to MRDWrapper.SetMinObject. The SetMinObject method on MultiRangeDownloaderWrapper is not thread-safe, and it can be called concurrently with read operations. Please see the detailed comment for the fix.

[codecov]

Codecov Report

❌ Patch coverage is 76.66667% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.01%. Comparing base (2cbaedd) to head (de6509a).

Files with missing lines	Patch %	Lines
internal/fs/inode/file.go	55.55%	2 Missing and 2 partials ⚠️
internal/gcsx/mrd_instance.go	80.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4259      +/-   ##
==========================================
- Coverage   83.06%   83.01%   -0.05%     
==========================================
  Files         156      156              
  Lines       19173    19187      +14     
==========================================
+ Hits        15926    15929       +3     
- Misses       2654     2662       +8     
- Partials      593      596       +3     

Flag	Coverage Δ
unittests	83.01% <76.66%> (-0.05%)	⬇️

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.