Re: [PATCH storage] lvm: bound storage status commands

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

A very small comment below.

Thomas Lamprecht <t.lamprecht@proxmox.com> writes:

> A status command stuck in uninterruptible device I/O blocks storage
> status indefinitely: run_command waits for its child, and SIGKILL does
> not end such a command. That happens for example when all paths of a
> multipath map that queues I/O are gone. pvestatd and API calls that

Only if queue_if_no_path is enabled or no_path_retry is set to queue for
the device.

One of the difficulties is that is not easy to detect the value of
either configuration at runtime. e.g. it could come from
/etc/multipath.conf, /etc/multipath/conf.d/*, from the defaults [1],
etc. On top of that, each location contains global values and device
specific rules and it is not always clear what takes priority over what.

[1] https://github.com/opensvc/multipath-tools/blob/0.11.1/libmultipath/hwtable.c

> query the storage status then hang with it.
>
> Run the vgs, lvs and pvscan calls of a status pass through a killable
> Perl child, with a 15-second limit including lock acquisition. The
> command inherits a lock and keeps it until it really exits, so later
> passes cannot pile up commands on the same failed device. While a
> command is stuck, LVM storages are reported inactive; other storages
> are not affected.
>
> Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> ---
>
> This was part of v3 of the SAN series [0], with a new run_command() mode
> and a hand-over across the daemon re-exec. It is independent of that
> series and could be remodelled so that the existing
> run_fork_with_timeout() is enough, and nothing changes for callers
> outside of a status pass.
>
> Tested on a nested cluster with a shared volume group on a two-path
> iSCSI LUN whose map queues forever, with stock libpve-common-perl: with
> all paths cut for three minutes, one killed vgs stays in state D as a
> child of init and holds the lock, pvestatd keeps running, and the
> storage status API keeps answering with the LVM storages inactive. A
> pass spends 14 seconds waiting for the lock; the rest of the 27 seconds
> per API call there are the portal checks of the unreachable iSCSI
> target, as before. Everything is active again at the first check ten
> seconds after the paths return.
>
> Known limits:
>
>  - The status commands of all callers on a node (pvestatd, API workers,
>    pvesm) now run one after the other. A caller whose wait for the lock
>    plus its own command exceed 14 seconds reports the LVM storages
>    inactive for that pass. vgs takes 0.05 seconds on the test nodes
>    with 36 PVs, so this needs very slow listings and several callers at
>    once.
>  - Activating an inactive thin pool (lvchange) is not bounded, as on
>    master: killing an activation midway would be worse.
>  - A SIGTERM that arrives while run_fork_with_timeout() waits is
>    consumed there, as for the df fork of directory storages today, where
>    the window is much shorter. Stopping pvestatd during that wait
>    therefore ends with the SIGKILL fallback of the stop command after
>    five seconds. That is for run_fork_with_timeout() to fix.
>  - The stuck command stays in the control group of the service that
>    started it. If that is pvestatd, systemctl stop and restart wait
>    for the command or for systemd's stop timeout (68 seconds in the
>    test, until the target's default queueing gave up); a reload
>    re-executes the daemon and does not wait. On master the daemon
>    hangs with the command in that situation.
>
> [0]: https://lore.proxmox.com/pve-devel/20261105233336.2178064-1-t.lamprecht@proxmox.com/
>
> [...]

Despite the limitations above this is an improvement over the current
situation where once all paths are offline (with no_path_retry=queue)
all pvedaemon workers would get stuck after a couple of seconds and,
e.g. it would not be possible to login to the web UI.

Tested-by: Maximiliano Sandoval <m.sandoval@proxmox.com>

-- 
Maximiliano