[00/15] Yet Another path checker refactor

Message

Benjamin Marzinski Aug. 28, 2024, 10:17 p.m. UTC

This patchset cleans up some of the ugly code from my last refactor, and
speeds up multipathd path checking, by refactoring how paths are
checked.

multipathd previously ran the checker on a path, waited briefly for it
to complete, and if it did, updated the path and the priorities for the
multipath device and possibly reloaded it.

This had multiple issues.
1. All the paths waited for their checkers to complete serially. This
wasted time with no benefit. It also meant that it wasn't that uncommon
for a checker to not finish in time, and get marked pending.

2. The prioritiers could get run on paths multiple times during a
checker loop if multiple paths were restored at the same time, which is
a common occurance.

3. In an effort to keep a multipath device's state consistent despite
the delays introduced by waiting for the path checkers, paths were
checked by multipath device. However checking the paths could involve
reloading a multipath device table, or even removing a multipath device.
This added some ugly backout and retry logic to the path check code.

With this patchset, the code now first starts the path checkers on all
devices due for a path check. Then it goes back and updates the state of
all the path devices, waiting at most a total of 1ms if there are
checkers that need waiting for. Once all of the paths have been updated,
it goes through each multipath device, updating path priorities and
reloading the device as necessary.

This allows multipathd to spend less time and do less redundant work
while checking paths, while making paths slightly more likely to not
spend a checker cycle marked as pending.

Since there isn't a delay waiting for the previous checker before
starting the next one, the path checker code has reverted to checking
all paths in pathvec order, instead of by multipath device. Updating the
paths in pathvec order simplifies the code, since the multipath devices
can change during path updates. Starting the checkers in pathvec order
keeps a path from having its checker started towards the end of the list
of paths but checking for the result towards the start of the list of
paths. However, it's possible to start the checkers by multipath device
without adding much complexity, if people think that the benfit of
starting the checkers as close together as possible outweighs the
benefit of giving each checker as close to the same amount of time as
possible to complete.

Benjamin Marzinski (15):
  libmultipath: store checker_check() result in checker struct
  libmultipath: add missing checker function prototypes
  libmultipath: split out the code to wait for pending checkers
  libmultipath: remove pending wait code from libcheck_check calls
  multipath-tools tests: fix up directio tests
  libmultipath: split get_state into two functions
  libmultipath: change path_offline to path_sysfs_state
  multipathd: split check_path_state into two functions
  multipathd: split do_checker_path
  multipathd: split check_path into two functions
  multipathd: split handle_uninitialized_path into two functions
  multipathd: split check_paths into two functions
  multipathd: update priority once after updating all paths
  multipathd: remove pointless check
  multipathd: fix deferred_failback_tick for reload removes

 libmultipath/checkers.c           |  35 +--
 libmultipath/checkers.h           |   8 +-
 libmultipath/checkers/directio.c  | 129 ++++++----
 libmultipath/checkers/tur.c       |  66 +++--
 libmultipath/discovery.c          |  90 ++++---
 libmultipath/discovery.h          |   6 +-
 libmultipath/libmultipath.version |   3 +-
 libmultipath/print.c              |   2 +-
 libmultipath/propsel.c            |   1 +
 libmultipath/structs.h            |  21 +-
 multipathd/main.c                 | 385 ++++++++++++++++++------------
 tests/directio.c                  | 133 +++++++----
 12 files changed, 546 insertions(+), 333 deletions(-)

Comments

Martin Wilck Sept. 4, 2024, 8:07 p.m. UTC | #1

On Wed, 2024-08-28 at 18:17 -0400, Benjamin Marzinski wrote:
> 

> With this patchset, the code now first starts the path checkers on
> all
> devices due for a path check. Then it goes back and updates the state
> of
> all the path devices, waiting at most a total of 1ms if there are
> checkers that need waiting for. Once all of the paths have been
> updated,
> it goes through each multipath device, updating path priorities and
> reloading the device as necessary.
> 
> This allows multipathd to spend less time and do less redundant work
> while checking paths, while making paths slightly more likely to not
> spend a checker cycle marked as pending.

while I've already responded to several of the patches: Thanks a lot
for this!! I've been thinking about a change like this for a long time,
and this is definitely a change for the better.

Martin