On 30/9/20 5:16 pm, Filipe Manana wrote:
On Wed, Sep 30, 2020 at 5:14 AM Anand Jain <anand.jain@xxxxxxxxxx> wrote:
On 30/9/20 1:26 am, Josef Bacik wrote:
On 9/29/20 12:13 PM, Filipe Manana wrote:
On Tue, Sep 29, 2020 at 5:02 PM Josef Bacik <josef@xxxxxxxxxxxxxx> wrote:
On 9/29/20 11:55 AM, Filipe Manana wrote:
On Tue, Sep 29, 2020 at 4:50 PM Anand Jain <anand.jain@xxxxxxxxxx>
wrote:
btrfs/064 aimed to test balance and replace concurrency while the
stress
test is running in the background.
However, as the balance and the replace operation are mutually
exclusive, so they can never run concurrently.
And it's good to have a test that verifies that attempting to run them
concurrently doesn't cause any problems, like crashes, memory leaks or
some sort of filesystem corruption.
For example btrfs/187, which I wrote sometime ago, tests that running
send, balance and deduplication in parallel doesn't result in crashes,
since in the past they were allowed to run concurrently.
I see no point in removing the test, it's useful.
My confusion was around whether this test was actually testing what we
think it should be testing. If this test was meant to make sure that
replace works while we've got load on the fs, then clearly it's not
doing what we think it's doing.
Given that neither the test's description nor the changelog mention
that it expects device replace and balance to be able to run
concurrently,
that errors are explicitly ignored and redirected to $seqres.full, and
we don't do any sort of validation after device replace operations, it
makes it clear to me it's a stress test.
Sure but I spent a while looking at it when it was failing being very
confused. In my mind my snapshot-stress.sh is a stress test, because
its meant to run without errors. The changelog and description are
sufficiently vague enough that it appeared that Eryu meant to write a
test that actually did a replace and balance at the same time. The test
clearly isn't doing that, so we need to update the description so it's
clear that's what's going on. And then I wanted to make sure that we do
in fact have a test that stresses replace in these scenarios, because I
want to make sure we actually test replace as well.
Not ripping it out is fine, but updating the description so I'm not
confused in a couple years when I trip over this again would be nice.
Thanks,
As of now, we have the following balance concurrency tests.
-----
028 balance and unlink fsstress concurrency [1]
060 balance and subvol ops concurrency with fsstress [2]
061 balance and scrub concurrency with fsstress [2]
062 balance and defrag concurrency with fsstress [2]
063 balance and remount concurrency with fsstress [2]
064 balance and replace concurrency with fsstress [2]
177 balance and resize concurrency
No, 177 does not test balance and resize concurrency.
It tests balance when a swap file exists. And the resize happens
(starts and ends) before setting the swap file and before doing the
balance.
You are right. Thanks for correcting.
-Anand
Thanks.
187 balance, send and dedupe concurrency
190 balance with qgroup
[1]
args=`_scale_fsstress_args -z \
-f write=10 -f unlink=10 \
-f creat=10 -f fsync=10 \
-f fsync=10 -n 100000 -p 2 \
-d $SCRATCH_MNT/stress_dir`
[2]
args=`_scale_fsstress_args -p 20 -n 100 $FSSTRESS_AVOID -d
$SCRATCH_MNT/stressdir`
-----
064 shall test balance with fsstress in the background. The replace
thread is kept out with the early check of BTRFS_FS_EXCL_OP in the kernel.
I am ok with the 064 headers updated, will send v2.
Also, it turns out that this test case helped to find a btrfs-progs bug.
Its patch [1] is sent to the ML.
[1] btrfs-progs: fix return code for failed replace start
Thanks, Anand
Josef
