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. > > In this case if we're ok with it exercising the exclusion path then I > think we at least need to update the comment at the beginning of the > test so it's clear what the purpose of the test is. And then we need to > make sure we do actually have a test where device replace is properly > exercised. I _think_ it is with btrfs/065 and some others, so just > updating the comment here would be enough. Thanks, > > Josef -- Filipe David Manana, “Whether you think you can, or you think you can't — you're right.”
