Re: [PATCH] fstests: delete btrfs/064 it makes no sense

[Josef Bacik <josef@toxicpanda.com>]

On 9/29/20 12:13 PM, Filipe Manana wrote:
> On Tue, Sep 29, 2020 at 5:02 PM Josef Bacik <josef@toxicpanda.com> wrote:
>>
>> On 9/29/20 11:55 AM, Filipe Manana wrote:
>>> On Tue, Sep 29, 2020 at 4:50 PM Anand Jain <anand.jain@oracle.com> 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,

Josef