Re: [Last-Call] Artart last call review of draft-ietf-alto-performance-metrics-17

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

 



Hi Christian,

On Tue, Oct 19, 2021 at 2:54 AM Christian Amsüss <christian@xxxxxxxxxxx> wrote:
Hello Richard,

On Mon, Oct 18, 2021 at 03:43:50PM -0400, Y. Richard Yang wrote:
> Good comment. The document gives the high-level grammar (1 line) at the
> beginning of Sec. 2.2.
> It looks that your suggestion is the write out the complete grammar upfront:

I was not so much looking for the details of the terms "where <stat>
MUST be one of the following" was fine) but for

a) naming the formal language this is in. Reading, I've guessed that the
  square brackets mean "optional" because I've seen that in DOS-time
  documentation, and it looks formal, but does not declare a language. 
 
  A formal language that could be used here and is common in RFCs would
  be ABNF.


I see. See below, as we can address them together.
 
b) the tying in of the output -- the the term "metric-identifier" is not
  used anywhere else, and RFC7285 uses a CostMetric type in the
  cost-metric field; I'd appreciate if these identifiers were somehow
  linked.
  This would be trivial had ALTO used CDDL to describe its JSON, as
  then there could be a line like `cost-metric //= metric-identifier`;
  maybe the `object { ... }` notation has something similar?

The object notation is higher granularity and hence cannot handle strings structure.
 
  RFC7285 generally does without constructing string identifier, so
  there is no ABNF in there.

Correct.
 
  If you go with the suggested colon structuring and have text for that,
  possibly there the optionality and string concatenation be described
  in ABNF (or just in words with an example), and the need for a formal
  language would go away here.


Good comment. How about the following:

Old Sec. 2.2
"Hence, each performance metric's identifier
   should indicate the statistic (i.e., an aggregation operation), to
   become
       <metric-identifier> ::= <metric-base-identifier> [ '-' <stat> ]
   where <stat> MUST be one of the following:
 "

=>

"Hence, this document extends the general US-ASCII alphanumeric cost metric strings (formally specified as the CostMetric type in Section 10.6 of [RFC7285]) as follows: A cost metric string (denoted <cost-metric>) consists of a base metric identifier string (denoted <metric-base-identifier>) followed by an optional statistics string (denoted <stat>), connected by the ASCII character colon (':', U+003A), if the statistics field exists. Examples of cost metric strings include "delay-ow", "delay-ow:min", "delay-ow:p99".

> "...  Note that some systems use quantile, which is in the range [0, 1].
> This document uses percentile to make the identifier easier to read. When
> there is a more common form for a given percentile, it is RECOMMENDED that
> the common form being used; that is, instead of p0, use min; instead of
> p50, use median; instead of p100, use max".

Fine with me.


Great.
 
> > * Allowing decimals into the cost metric identifier introduces a dot which
> >   is reserved as per RFC7285 Section 10.6.
>
> Yes. Correct. From the grammar above, we made sure that we would not
> introduce a dot. We can add a sentence to point out that when choosing the
> base, we should follow this. Should we do this?

I think it'd be easiest (with the grammar or without) to to introduce
the number as a nonnegative integer. (That'd allow dropping mentions of
the minus and exp components).

Agree.
 
> >   A way out could be to formalize this structure and register the
> >   metric-base-identifiers for use with and without a stat parameter
> >   following the colon (instead of the dash).
>
> So the suggestion is that ":" as the consistent internal structure
> separator. This is quite reasonable.

Just beware that this is formally an update to the registry; not sure
whether that incurs an "updates" to 7285.


No. This will not update 7285. Sec. 10.6 of RFC7285 requires registration and we will do so.

 
> >   A few words on which statistic can be used with which metric could
> >   also help with bw-maxres. (What does bw-maxres-p50 mean, is it
> >   meaningful at all?)
>
> Mathematically, any percentile of a set of a single value is the value
> itself. But it is indeed a good idea to clarify it. We will add a sentence
> at the end of 2.2
>
> => Note that although one can use generic statistics (i.e., any percentile
> in [0, 100]) and multiple specifications may give the same value, it helps
> to choose the more intuitive and robust definition. For example, when the
> set is expected to be a single value. The max operator is more robust and
> hence recommended.

That sounds more confusing to me (the max operator ... so I should use
bw-maxres-max?); maybe (if correct) something like

| Note that unlike the other metrics, maxres is a single value and not
| sampled over time. Thus, it MUST NOT be specialized with a stat
| indicator, but is to be used in the base form.

Sounds good to me. We will use your wording.
 
> > * "Content-Length: TBA": Does this add value to the examples?
>
> We will add the final exact value when publishing, as it is part of HTTP
> header.

Sure it is part of the HTTP header, but so are many other headers
(Accept-Content-Encoding, User-Agent, what so not). Accept and
Content-Type add to the understanding, Content-Length will IMO be
ignored by all readers anyway.


OK. We will add the final value now.
 
> It is an example to illustrate both ipv4 and ipv6. If we do ipv4->ip4 and
> ipv6-ipv6, we will need two examples, and it takes more space.

Does it need examples of all of them? These are not unit tests, they are
illustrative.

Indeed. They are illustrative and we can drop the mix use to avoid confusion.
 
> > * "the -<percentile> component": "any 'stat' component"?
>
> Not sure what this is? Any more specific locator?

3.1.3

> Very good comment! We have made the changes.  The newer version fixed this
> issue.
> Please see -18 which is just loaded today.

Didn't see that at first ... something was changed in the uplaod process
and now there's no HTML version any more?


We will upload a newer version to address all issues soon and then I will debug to make sure that is an HTML version.

Thank you so much!
Richard

 
BR
c

--
To use raw power is to make yourself infinitely vulnerable to greater powers.
  -- Bene Gesserit axiom

-- 
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call

[Index of Archives]     [IETF Annoucements]     [IETF]     [IP Storage]     [Yosemite News]     [Linux SCTP]     [Linux Newbies]     [Mhonarc]     [Fedora Users]

  Powered by Linux