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

cluster: Addition of new Scrubbing test #246

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

amathuria
Copy link

Signed-off-by: Aishwarya Mathuria [email protected]

@amathuria amathuria force-pushed the wip-cbt-addscrubtest branch 3 times, most recently from 2071234 to 52e8dae Compare February 8, 2022 15:06
@amathuria amathuria marked this pull request as ready for review February 8, 2022 15:07
cluster/ceph.py Outdated Show resolved Hide resolved
Add 2 new tests similar to the background Recovery test for running Scrub load with client IO and the other one for running Scrub, Recovery, and Client load.
The Scrub and client IO test performs the following steps:
	- Create a pool and image to populate scrub objects (scrub pool)
	- Create scrub thread
	- Populate the scrub pool with objects using radosbench
	- Initiate deep-scrub on the scrub pool
	- Create a second pool and an image in it to run client IO
	- Initiate fio job on the second image at the same time the deep-scrub starts

In the second test, we have an additional recovery pool that is populated after an OSD is marked down and out.  Once the pool is populated we mark the OSD up and in which starts backfill. At the same time, we begin deep-scrub on the scrub pool and client IO.

Signed-off-by: Aishwarya Mathuria <[email protected]>
Copy link
Contributor

@sseshasa sseshasa left a comment

Choose a reason for hiding this comment

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

For the scrubbing test, the name of the states like 'osdout' and 'osdin' can confuse other readers. Can this be changed to more appropriate states like for e.g., 'fillscrubpool', 'initscrub' to make it more intuitive?

cluster/ceph.py Outdated Show resolved Hide resolved
Copy link
Contributor

@sseshasa sseshasa left a comment

Choose a reason for hiding this comment

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

I forgot to add this. The scrub tests can be factored out into a separate file under the same directory to avoid ceph.py from bloating up.

@amathuria
Copy link
Author

amathuria commented Aug 23, 2022

For the scrubbing test, the name of the states like 'osdout' and 'osdin' can confuse other readers. Can this be changed to more appropriate states like for e.g., 'fillscrubpool', 'initscrub' to make it more intuitive?

Makes sense, I'll work on that and shift the scrub tests into a separate file.

@@ -80,9 +82,17 @@ def initialize(self):

common.sync_files('%s/*' % self.run_dir, self.out_dir)

if 'scrubbing_test' in self.cluster.config:
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this just be called 'scrub_test'. I think all instances of 'scrubbing' in function names and other places can be replaced with 'scrub'. This makes the code much neater and consistent to read. What do you think?

@@ -231,6 +267,9 @@ def recovery_callback_blocking(self):
def recovery_callback_background(self):
logger.info('Recovery thread completed!')

def scrubbing_callback(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

@@ -114,6 +116,15 @@ def create_rbd_recovery(self):
rbd_name = '%s-%s' % (self.pool, self.get_rbd_name(node, ep_num))
self.cluster.mkimage(rbd_name, self.endpoint_size, self.pool, self.data_pool, self.order)

def create_rbd_scrubbing(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'create_rbd_scrub_pool()' sounds better.

@@ -45,3 +45,6 @@ def remove(self):

def create_recovery_image(self):
pass

def create_scrubbing_image(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub' here and in the other *_endpoint.py files.

cluster/ceph.py Outdated
@@ -9,6 +9,7 @@
import json

from .cluster import Cluster
from .scrub_tests import ScrubbingTestThreadBackground, ScrubRecoveryThreadBackground
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks much better now. To be consistent you can consider changing class name from "ScrubbingTestThreadBackground" to "ScrubTestThreadBackground".

cluster/ceph.py Outdated
@@ -563,6 +578,44 @@ def check_health(self, check_list=None, logfile=None, recstatsfile=None):

return ret

def log_scrubbing_stats(self, scrubstatsfile=None, pgid=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

cluster/ceph.py Outdated
@@ -654,6 +731,21 @@ def create_recovery_test(self, run_dir, callback, test_type='blocking'):
self.rt = RecoveryTestThreadBackground(rt_config, self, callback, self.stoprequest, self.haltrequest, self.startiorequest)
self.rt.start()

def create_scrubbing_test(self, run_dir, callback):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

cluster/ceph.py Outdated
@@ -672,6 +764,32 @@ def wait_recovery_done(self):
break
self.rt.join(1)

def maybe_populate_scrubbing_pool(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

cluster/ceph.py Outdated
common.pdsh(settings.getnodes('head'), 'sudo %s -p %s bench %s write -b %s --max-objects %s --no-cleanup' % (self.rados_cmd, self.scrub_pool_name, self.prefill_scrub_time, self.prefill_scrub_object_size, self.prefill_scrub_objects)).communicate()
#self.check_health()

def initiate_scrubbing(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

cluster/ceph.py Outdated
logger.info("Initiating scrub on pool %s" % self.scrub_pool_name)
common.pdsh(settings.getnodes('head'), '%s osd pool deep-scrub %s' % (self.ceph_cmd, self.scrub_pool_name)).communicate()

def wait_scrubbing_done(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

benchmark/fio.py Outdated
@@ -239,6 +263,12 @@ def recovery_callback_blocking(self):
def recovery_callback_background(self):
logger.info('Recovery thread completed!')

def scrubbing_callback(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

'scrubbing' to 'scrub'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants