Skip to content
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

qa: wait for file to have correct size #51995

Merged
merged 1 commit into from Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 26 additions & 16 deletions qa/tasks/cephfs/mount.py
Expand Up @@ -935,7 +935,7 @@ def open_no_data(self, basename):
))
p.wait()

def open_background(self, basename="background_file", write=True):
def open_background(self, basename="background_file", write=True, content="content"):
"""
Open a file for writing, then block such that the client
will hold a capability.
Expand All @@ -952,12 +952,11 @@ def open_background(self, basename="background_file", write=True):
import time

with open("{path}", 'w') as f:
f.write('content')
f.write("{content}")
f.flush()
f.write('content2')
while True:
time.sleep(1)
""").format(path=path)
""").format(path=path, content=content)
else:
pyscript = dedent("""
import time
Expand All @@ -973,7 +972,10 @@ def open_background(self, basename="background_file", write=True):
# This wait would not be sufficient if the file had already
# existed, but it's simple and in practice users of open_background
# are not using it on existing files.
self.wait_for_visible(basename)
if write:
self.wait_for_visible(basename, size=len(content))
else:
self.wait_for_visible(basename)

return rproc

Expand Down Expand Up @@ -1011,19 +1013,27 @@ def wait_for_dir_empty(self, dirname, timeout=30):
if nr_links == 2:
return

def wait_for_visible(self, basename="background_file", timeout=30):
def wait_for_visible(self, basename="background_file", size=None, timeout=30):
i = 0
args = ['stat']
if size is not None:
args += ['--printf=%s']
args += [os.path.join(self.hostfs_mntpt, basename)]
while i < timeout:
r = self.client_remote.run(args=[
'stat', os.path.join(self.hostfs_mntpt, basename)
], check_status=False)
if r.exitstatus == 0:
log.debug("File {0} became visible from {1} after {2}s".format(
basename, self.client_id, i))
return
else:
time.sleep(1)
i += 1
p = self.client_remote.run(args=args, stdout=StringIO(), check_status=False)
if p.exitstatus == 0:
if size is not None:
s = p.stdout.getvalue().strip()
if int(s) == size:
log.info(f"File {basename} became visible with size {size} from {self.client_id} after {i}s")
return
else:
log.error(f"File {basename} became visible but with size {int(s)} not {size}")
else:
log.info(f"File {basename} became visible from {self.client_id} after {i}s")
return
time.sleep(1)
i += 1

raise RuntimeError("Timed out after {0}s waiting for {1} to become visible from {2}".format(
i, basename, self.client_id))
Expand Down
20 changes: 10 additions & 10 deletions qa/tasks/cephfs/test_client_recovery.py
Expand Up @@ -7,7 +7,9 @@
from textwrap import dedent
import time
import distutils.version as version
import random
import re
import string
import os

from teuthology.orchestra import run
Expand Down Expand Up @@ -217,8 +219,10 @@ def _test_stale_caps(self, write):
# Capability release from stale session
# =====================================
if write:
cap_holder = self.mount_a.open_background()
content = ''.join(random.choices(string.ascii_uppercase + string.digits, k=16))
cap_holder = self.mount_a.open_background(content=content)
else:
content = ''
self.mount_a.run_shell(["touch", "background_file"])
self.mount_a.umount_wait()
self.mount_a.mount_wait()
Expand All @@ -229,7 +233,7 @@ def _test_stale_caps(self, write):

# Wait for the file to be visible from another client, indicating
# that mount_a has completed its network ops
self.mount_b.wait_for_visible()
self.mount_b.wait_for_visible(size=len(content))

# Simulate client death
self.mount_a.suspend_netns()
Expand Down Expand Up @@ -260,11 +264,9 @@ def _test_stale_caps(self, write):
"Capability handover took {0}, expected approx {1}".format(
cap_waited, session_timeout
))

self.mount_a._kill_background(cap_holder)
finally:
# teardown() doesn't quite handle this case cleanly, so help it out
self.mount_a.resume_netns()
self.mount_a.resume_netns() # allow the mount to recover otherwise background proc is unkillable
self.mount_a._kill_background(cap_holder)

def test_stale_read_caps(self):
self._test_stale_caps(False)
Expand Down Expand Up @@ -315,9 +317,9 @@ def test_evicted_caps(self):
cap_waited, session_timeout / 2.0
))

self.mount_a._kill_background(cap_holder)
finally:
self.mount_a.resume_netns()
self.mount_a.resume_netns() # allow the mount to recover otherwise background proc is unkillable
self.mount_a._kill_background(cap_holder)

def test_trim_caps(self):
# Trim capability when reconnecting MDS
Expand Down Expand Up @@ -383,7 +385,6 @@ def test_filelock(self):

self.mount_b.check_filelock(do_flock=flockable)

# Tear down the background process
Copy link
Contributor

Choose a reason for hiding this comment

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

removal of these comments is intentional right? Looks too trivial

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. They're ultra-redundant.

self.mount_a._kill_background(lock_holder)

def test_filelock_eviction(self):
Expand Down Expand Up @@ -412,7 +413,6 @@ def test_filelock_eviction(self):
# succeed
self.wait_until_true(lambda: lock_taker.finished, timeout=10)
finally:
# Tear down the background process
self.mount_a._kill_background(lock_holder)

# teardown() doesn't quite handle this case cleanly, so help it out
Expand Down