test(rapid appends): appends with concurrent file handles must be real time #3545
Conversation
There was a problem hiding this comment.
Summary of Changes
Hello @anushka567, I'm Gemini Code Assist1! 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 introduces a new suite of integration tests designed to validate the real-time visibility and concurrent access behavior of rapid appends in GCS FUSE. The primary goal is to ensure that data appended to unfinalized objects is immediately readable and consistent across different file handles and mounts, addressing the requirements outlined in the associated bug b/432420125.
Highlights
- New Integration Test Suite: I've added a dedicated integration test suite (
unfinalized_appends) to thoroughly verify the functionality and real-time behavior of rapid appends in GCS FUSE. - Real-time Append Visibility: The new tests confirm that data appended to unfinalized objects is immediately visible, even when accessed from different GCS FUSE mounts configured for rapid appends and zero metadata cache TTL. This addresses the 'real time' requirement from the PR title.
- Concurrent Handle Behavior: The suite includes tests for scenarios where multiple file handles (opened in append mode or read-write mode) are concurrently used on the same object. This verifies append session takeover and ensures real-time data consistency across concurrent access points.
- Visibility of Non-Append Writes: I've included tests that distinguish between data appended using
os.O_APPEND(which should be real-time visible) and data written at an offset in non-append mode (os.O_WRONLY), which is expected to be visible only after the file handle is closed.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| 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 issue 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 is currently in preview and 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 to provide feedback.
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
-
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. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test suite, unfinalized_appends, to verify the real-time visibility of appends with concurrent file handles. The changes include adding the new test package to the test runner script and implementing the test logic with its setup. My review focuses on improving code clarity and fixing a few issues, including typos, a potential bug in error handling, and a compilation error. The overall structure of the new tests is sound and follows existing patterns in the repository.
tools/integration_tests/unfinalized_appends/unfinalized_appends_test.go
Outdated
Show resolved
Hide resolved
tools/integration_tests/unfinalized_appends/unfinalized_appends_test.go
Outdated
Show resolved
Hide resolved
tools/integration_tests/unfinalized_appends/unfinalized_appends_test.go
Outdated
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3545 +/- ##
==========================================
+ Coverage 78.84% 79.73% +0.89%
==========================================
Files 140 141 +1
Lines 18600 18931 +331
==========================================
+ Hits 14665 15095 +430
+ Misses 3445 3326 -119
- Partials 490 510 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9e85d8a to
b88a4b5
Compare
cfd5eee to
a5193c1
Compare
dd99231 to
68a070c
Compare
a5193c1 to
9739ff2
Compare
|
|
||
| // Read from back-door to validate visibility on GCS. | ||
| expectedContent := t.fileContent + initialContent + appendContent | ||
| contentAfterClose, err := client.ReadObjectFromGCS(ctx, storageClient, path.Join(testDirName, t.fileName)) |
There was a problem hiding this comment.
nit: contentAfterClose -> contentAfterSync ?
Description
The functional test in this PR tests the scenario when multiple concurrent file handles are opened against the same unfinalized object from the same mount( i.e. the same file inode).
The test includes :
Link to the issue in case of a bug fix.
b/432420125
Testing details
--- PASS: TestRapidAppendsSuite/TestAppendsVisibleInRealTimeWithConcurrentRPlusHandle (1.46s)Any backward incompatible change? If so, please explain.