Re: [PATCH v2] btrfs-progs: add warning for mixed profiles filesystem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi David,

On 4/1/20 8:20 PM, David Sterba wrote:
On Wed, Apr 01, 2020 at 07:15:19PM +0200, Goffredo Baroncelli wrote:
On 4/1/20 12:05 AM, Zygo Blaxell wrote:
On Tue, Mar 31, 2020 at 10:46:17PM +0100, Graham Cobb wrote:
On 31/03/2020 20:10, Goffredo Baroncelli wrote:
WARNING: ------------------------------------------------------
WARNING: Detection of multiple profiles for a block group type:
WARNING:
WARNING: * DATA ->          [raid1c3, single]
WARNING: * METADATA ->      [raid1, single]
WARNING:
WARNING: Please consider using 'btrfs balance ...' commands set
WARNING: to solve this issue.
WARNING: ------------------------------------------------------

The check is a good a idea but I think the warning is too strong. I
would prefer that the word "Warning" is reserved for cases and
operations that may actually damage data (such as reformating a
filesystem). [Note: in a previous job, my employer decided that the word
Warning was ONLY to be used if there was a risk of harm to a human - for
example, electrical safety]

In some fields, like medical devices, terms "warning", "caution", "notice"
are strictly regulated; and your employer is right: warning is
required only when an human harm could happen.

In btrfs however, the rules are a bit relaxed; so we have only ERROR (fatal)
and Warning (which is more like caution).

However I think that an unexpected profile is to be considered a severe fault.

I agree with the 'unexpected' part and as I read the other feedback, but
the mixed profiles do not emerge out of nowhere. It requires a balance
operation and that it changes the filesystem layout I consider expected.

If the balance+conversion operation ends without problem AND if it
converts all the filesystem, a "mixed" profile doesn't appear.
However for the other case like:
- an interrupt of balance process for an external reasons (which likely
get the attention of the user, until he forgets to complete the process)
- a balance made with some filter (like -convert=xxx,usage=yyy)

The results is a mixed profile. To avoid that the user has to do a lot
of attention.
Moreover if you watch the output of a "fi us" (or "dev us") you cannot
understand which profile is representative of the filesystem. (see below)



I.e. you could have a RAID5 instead of a RAID1, where the former is more
fragile than the latter.
Moreover I suspect that a lot of problems reported on mailing list born from
a mixed profile filesystem...

Do you have an example? Converting between some priofiles is tricky, eg.
when the striped ones need enough space on all devices, unlike mirrored
that need that on 2/3/4 devices. But that's not something I feel is
related to lack of the warning.

Read this more as sensation than a true fact.
However what seemed strange to me was the fact that if you have a raid6 profile,
and convert it to a raid1 without completing the process. Then the next chunk will
be a raid6. Other case: you have a raid1 profile, then convert it to a raid6 but not
complete the process. Then the next chunk will be a raid6.
raid6 wins always... [*] This is not what an user expects; I expect that
the latest balance profile wins...

[*] See the thread "Question: how understand the raid profile of a btrfs filesystem".


Also, btrfs fi usage is something that I routinely run continuously in a
window (using watch) when a remove/replace/balance operation is in

I was going to say please put all the new output lines at the bottom,

The added lines are the last ones. Do you mean top ?

so that 'watch' windows can be minimally sized without having to write
something like

	watch 'btrfs fi usage /foo | sed -e "g/WARNING:/d"'

People with short terminal windows running btrfs fi usage directly from
the command line would probably complain about extra lines at the bottom...

Another good idea here would be a --quiet switch, or
'--no-profile-warning'.

I think that having a global switch like '--no-profile-warning' is a good thing.


progress to monitor at a glance what is happening - I don't want to
waste all that space on the screen. To say nothing of the annoyance of
having it shouting at me for weeks on end while **I AM TRYING TO FIX THE
DAMN PROBLEM!**.

I would suggest a more compact layout and factual tone. Something like:

Caution: This filesystem has multiple profiles for a block group type
so new block groups will have unpredictable profiles.
   * DATA ->          [raid1c3, single]
   * METADATA ->      [raid1, single]
Use of 'btrfs balance' is recommended as soon as possible to move all
blocks to a single profile for each of data and metadata.

How about a one-liner:

	NOTE: Multiple profiles detected.  See 'man btrfs-filesystem'.


WARNING: Multiple profiles detected.  See 'man btrfs-filesystem'.
WARNING: data -> [raid1c3, single], metadata -> [raid1, single]

I think that the one above could be a good compromise.

I agree, though I'd prefer only the reference to manual page with enough
examples and commandds to see what it means and what to do next.

The output style of the tools I generally try to maintain is to provide
building blocks or combining information from various sources together,
not necessarily a tool that comes with warning everywhere a first time
user can be surprised.

I am thinking also to combine '--no-profile-warning' and '--verbose' to have
three different warning level (--no-profile-warning -> no warning at all, nothing
-> small warning, --verbose -> "giant" warning).

That sounds like an overkill to me, the option is too long and I think
slightly experienced users would have to use it all the time.

My feeling is that in the 99% of the case the user doesn't see the warning. So
he doesn't need to use --no-profile-warning.
For the other cases something strange is happening. And it is better a warning
than forget a "mixed profile". I thought the switch more for a script than
an "human" user.

May be that we should ensure that the warning doesn't appear when
a balance is in progress. But only when a balance is paused/finished/stopped.
Unfortunately this seems to require root privileges.

If we
could find commands where the warning makes most sense, like right
before or after balance, then it's fine to add it to the output. But for
'fi df' or 'fi us' it's IMHO inappropriate.

My feeling is that a mixed profile is the result of not appropriate
use of "btrfs bala". So in a standard condition the warning should not
appear.
In a non standard condition (like a balance to gain some space) the user
should have all the support to avoid any error.
This is the reason of the "WARNING".


This is all usability tuning, so I'm open to discussion or
disagreements, we'll find some solution.

Adding or removing a check is simple; so for me it is ok to starts with
the agreed commands and then extend to other if we want.

I will work at the man page. I think that on this point we all agree.


BR
G.Baronelli

--
gpg @keyserver.linux.it: Goffredo Baroncelli <kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux