[dm-devel] [PATCH] multipath-tools: add info to man page for the historical-service-time path selector

[Martin Wilck]

On Tue, 2020-07-07 at 17:44 -0400, Gabriel Krisman Bertazi wrote:
> Xose Vazquez Perez <xose.vazquez at gmail.com> writes:
> 
> > Cc: Khazhismel Kumykov <khazhy at google.com>
> > Cc: Gabriel Krisman Bertazi <krisman at collabora.com>
> > Cc: Martin Wilck <mwilck at suse.com>
> > Cc: Benjamin Marzinski <bmarzins at redhat.com>
> > Cc: Christophe Varoqui <christophe.varoqui at opensvc.com>
> > Cc: DM-DEVEL ML <dm-devel at redhat.com>
> > Signed-off-by: Xose Vazquez Perez <xose.vazquez at gmail.com>
> > ---
> >  multipath/multipath.conf.5 | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/multipath/multipath.conf.5
> > b/multipath/multipath.conf.5
> > index 05a5e8ff..6e637769 100644
> > --- a/multipath/multipath.conf.5
> > +++ b/multipath/multipath.conf.5
> > @@ -205,6 +205,10 @@ of outstanding I/O to the path.
> >  (Since 2.6.31 kernel) Choose the path for the next bunch of I/O
> > based on the amount
> >  of outstanding I/O to the path and its relative throughput.
> >  .TP
> > +.I "historical-service-time 0"
> > +(Since 5.8 kernel) Choose the path for the next bunch of I/O based
> > on the shortest
> > +time by comparing estimated service time (based on historical
> > service
> > time).
> Hi,
> 
> Thanks for doing this.
> 
> What about:
> 
> Choose the path for the next bunch of IOs through an estimation of
> future service time based on the history of previous I/O submitted to
> each path, in an attempt to maximize throughput.  A path's service-
> time
> is loosely defined as the time between an IO start and its completion
> and is updated through an exponential moving average (EMA) of the
> historical service time of each path.

This is a too lengthy compared to the text we provide for the other
policies. I would suggest using just the first sentence:

> (Since 5.8 kernel) Choose the path for the next bunch of IOs through
an estimation of
> future service time based on the history of previous I/O submitted to
> each path.

If you want to provide more detail, add a link to the documentation of
the kernel feature, or provide a separate man page. multipath.conf(5)
is too big already.

> It supports some parameters, shouldn't they be documented here? 

Yes, definitely. Please add the parameter definitions. 

>  Some
> explanation for the parameters exists in hst_create() in the kernel
> 
>  /*
>   * Arguments: [<base_weight> [<threshold_multiplier>]]
>   *   <base_weight>: Base weight for ema [0, 1024) 10-bit fixed
> point. A
>   *                  value of 0 will completely ignore any history.
>   *                  If not given, default (HST_FIXED_95) is used.
>   *   <threshold_multiplier>: Minimum threshold multiplier for paths
> to
>   *                  be considered different. That is, a path is
>   *                  considered different iff (p1 > N * p2) where p1
>   *                  is the path with higher service time. A
> threshold
>   *                  of 1 or 0 has no effect. Defaults to 0.
>   */

This is way too technical, hard to understand even for developers. What
is HST_FIXED_95? What is the time unit for EMA (IOW, can we relate the
base weight to a half-life time somehow)? Why would I want to use N !=
0, what effect does it have in practice? I guess we should simply
provide the parameter names and recommended values to give users a head
start, and provide a link to a description of the algorithm for users
who want to know the details.

Please send an updated patch.

Martin