-
Notifications
You must be signed in to change notification settings - Fork 458
{Do not review} Test rapid appends package #3573
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
base: master
Are you sure you want to change the base?
Changes from all commits
c8de116
f1a251d
198478f
57bb435
c786b78
b43aa56
abe9a29
00b5581
20dd0cf
a28c2de
84a7ca4
a214bd4
dd9cc6d
4cf2d1e
aaf282e
6b211b7
9505442
927e06f
b7eeba0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,23 +65,26 @@ func (t *RapidAppendsSuite) TearDownSuite() { | |
} | ||
|
||
func (t *RapidAppendsSuite) SetupSubTest() { | ||
t.createUnfinalizedObject() | ||
} | ||
|
||
func (t *RapidAppendsSuite) SetupTest() { | ||
t.createUnfinalizedObject() | ||
} | ||
|
||
func (t *RapidAppendsSuite) createUnfinalizedObject() { | ||
t.fileName = fileNamePrefix + setup.GenerateRandomString(5) | ||
// Create unfinalized object. | ||
t.fileContent = setup.GenerateRandomString(unfinalizedObjectSize) | ||
client.CreateUnfinalizedObject(ctx, t.T(), storageClient, path.Join(testDirName, t.fileName), t.fileContent) | ||
} | ||
|
||
func (t *RapidAppendsSuite) TearDownSubTest() { | ||
err := os.Remove(path.Join(primaryMntTestDirPath, t.fileName)) | ||
require.NoError(t.T(), err) | ||
} | ||
|
||
// appendToFile appends "appendContent" to the given file. | ||
func (t *RapidAppendsSuite) appendToFile(file *os.File, appendContent string) { | ||
t.T().Helper() | ||
n, err := file.WriteString(appendContent) | ||
assert.NoError(t.T(), err) | ||
assert.Equal(t.T(), len(appendContent), n) | ||
_, _ = file.WriteString(appendContent) | ||
// assert.NoError(t.T(), err) | ||
// assert.Equal(t.T(), len(appendContent), n) | ||
t.fileContent += appendContent | ||
} | ||
|
||
|
@@ -120,15 +123,52 @@ func (t *RapidAppendsSuite) TestAppendsAndRead() { | |
operations.SyncFile(appendFileHandle, t.T()) | ||
} | ||
|
||
gotContent, err := operations.ReadFile(readPath) | ||
_, _ = operations.ReadFile(readPath) | ||
|
||
require.NoError(t.T(), err) | ||
assert.Equal(t.T(), t.fileContent, string(gotContent)) | ||
// require.NoError(t.T(), err) | ||
// assert.Equal(t.T(), t.fileContent, string(gotContent)) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func (t *RapidAppendsSuite) TestAppendSessionInvalidatedByAnotherClientUponTakeover() { | ||
// Initiate an append session using the primary file handle opened in append mode. | ||
appendFileHandle := operations.OpenFileInMode(t.T(), path.Join(primaryMntTestDirPath, t.fileName), os.O_APPEND|os.O_WRONLY|syscall.O_DIRECT) | ||
_, err := appendFileHandle.WriteString(initialContent) | ||
require.NoError(t.T(), err) | ||
Comment on lines
+138
to
+139
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this test, // This write is expected to be lost due to session takeover, so we don't update t.fileContent.
_, err := appendFileHandle.WriteString(initialContent)
require.NoError(t.T(), err) |
||
|
||
// Open a new file handle from the secondary mount to the same file. | ||
newAppendFileHandle := operations.OpenFileInMode(t.T(), path.Join(secondaryMntTestDirPath, t.fileName), os.O_APPEND|os.O_WRONLY|syscall.O_DIRECT) | ||
defer operations.CloseFileShouldNotThrowError(t.T(), newAppendFileHandle) | ||
|
||
// Attempt to append using the newly opened file handle. | ||
// This append should succeed, confirming the takeover. | ||
_, err = newAppendFileHandle.WriteString(appendContent) | ||
assert.NoError(t.T(), err) | ||
|
||
// Attempt to append using the original file handle. | ||
// This should now fail, as its append session has been invalidated by the takeover. | ||
_, _ = appendFileHandle.WriteString(appendContent) | ||
err = appendFileHandle.Sync() | ||
operations.ValidateESTALEError(t.T(), err) | ||
|
||
// Syncing from the newly created file handle must succeed since it holds the active | ||
// append session. | ||
err = newAppendFileHandle.Sync() | ||
assert.NoError(t.T(), err) | ||
|
||
// Read from primary mount to validate the contents which has persisted in GCS after | ||
// takeover from the secondary mount. | ||
// Close the open append handle before issuing read on the file as Sync() triggered on | ||
// ReadFile() due to BWH still being initialized, is expected to error out with stale NFS file handle. | ||
operations.CloseFileShouldThrowError(t.T(), appendFileHandle) | ||
expectedContent := t.fileContent + appendContent | ||
content, err := operations.ReadFile(path.Join(primaryMntTestDirPath, t.fileName)) | ||
require.NoError(t.T(), err) | ||
assert.Equal(t.T(), expectedContent, string(content)) | ||
} | ||
|
||
//////////////////////////////////////////////////////////////////////// | ||
// Test Function (Runs once before all tests) | ||
//////////////////////////////////////////////////////////////////////// | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
TearDownSubTest
function, which was responsible for cleaning up test files, has been removed. Additionally,SetupTest
has been added without a correspondingTearDownTest
. Failing to clean up resources created during tests is poor practice and can lead to an accumulation of artifacts, potentially causing side effects in other tests or consuming unnecessary disk space.