Subject: Artart telechat review of draft-ietf-alto-multi-cost-08

From: Martin Thomson <[email-masked]>
Date: Wed, 05 Apr 2017 19:07:34 -0700

Reviewer: Martin Thomson
Review result: Ready with Issues

Document: draft-ietf-alto-multi-cost-08
Date: 2017-04-06
Reviewer: Martin Thomson

This document describes how ALTO can be used to acquire cost maps with
multiple
cost metric instead of a single metric.

The document very carefully deals with backwards compatibility with
existing
ALTO servers and clients.  I don't anticipate many issues arising from
the
deployment of this protocol.

I started reading -07, but finished with -08.  I checked that the
issues I raise
still exist, but I'm not infallible.  Apologies if I get something
wrong.

Minor issues: I've identified a few issues that are more than nits,
these are
marked "IMPORTANT" below.

The abstract includes considerable justificiation.  An abstract only
needs to
describe the *what*, not the *why*.  Thus, what is there could be
simplified
considerably, e.g.,

   This document defines a new method for retrieving multiple cost
metrics in a
   single request for an ALTO filtered cost map.  It also defines
improvements
   to the constraints that can be used for filtered cost maps.

Section 1

The introduction uses a bunch of odd terms.  Some of these are
recognizable from
the ALTO specification, but some of the jargon seems unnecessary.  In
particular, "Internet View", "Provider Network region" and "Vector
costs".  All
of which I think that I understand, but they make the doc hard to
follow.

Generally, I found the introduction quite hard to follow, both for
that reason
and structurally.  The introduction could be a lot shorter and more
concise:

1. ALTO defines multiple cost types (and more are being defined).
2. Clients sometimes consume multiple cost types.
3. Requesting multiple cost types at the same time is more efficient
(for
   several reasons).
4. This document defines how to do that.
5. Separately, when multiple cost types are present, more
sophisticated
   filtering can improve efficiency further.
6. This document defines how to do that too.

Section 2

There are several items in the list here that are not used:
Application Client,
Network Service Provider, maybe more.  Please check and remove those
that don't apply.

The RFC 7285 section reference thing is unnecessary.

This document doesn't cite RFC 2119, but it uses the keywords.

Section 3.1

The example shows an empty cost-type, but the schema you define allows
it to be
absent.  You REALLY need to pick one.  I don't believe that this is a
compatibility issue: once you have determined that a client supports
multi-cost,
then you can do anything you like, just be clear about it.

Section 3.2

I found the argument about the ease of writing a parser to be quite
unconvincing.  However, a new media type that is largely the same as
the
existing media type won't necessarily result in code duplication.

Just say what it is you expect to happen and don't try to be
apologetic about
it.  What you have appears to be a workable design.

Section 3.5

This section is confusing.  You only need to say that you are not
altering full
cost map resources in any way and that clients need to use filtered
cost maps if
they want multiple costs at the same time.  (Obviously you could have,
but
creating multiple resources with the full combinatorial mess caused by
combining
many cost types is unwieldy.)  At a minimum, the second paragraph here
can be
removed.

Section 3.6.2

IMPORTANT: You don't define what happens when a client provides
"or-constraints"
and "constraints" at the same time.  There are several valid options,
but you
need to choose.

Section 3.6.3

It is probably worth explicitly noting that if "testable-cost-types"
does not
include values from "multi-cost-types", then those types can't be
included in
"constraints"/"or-constraints".

Please explain the default value for the index for the
"constraints"/"or-constraints" express in this section in addition to
where it
is hidden in a note later in the document.

Section 3.6.5

Uppercase for "must not" in the second paragraph.  (The "may" later in
the
paragraph might be better as "can".)

In the example, the resource named "filtered-multicost-map" is
provided for
legacy reasons only.  Why bother including "max-cost-types" and
"cost-type-names" at all when "filtered-cost-map-extended" includes
all that and
more?

Section 4.1.1

The definition of the schema here (and later) actually redefines the
object
completely.  I found that confusing initially.  It would be good to
identify the
*changes* from the base specification somehow.

Can testable-cost-type-names be present and empty if cost-constraints
is false?
The first part of the definition permits that, the second forbids it.

Section 4.1.2

The redefinition of PIDFilter is unnecessary.

IMPORTANT: pids is optional in RFC 7285.  Why the change?

I find the redefinition of the optionality of cost-type to be worthy
of special
note.

In the definition of "or-constraints", you use a "database query"
where words
would suffice.

Section 4.1.3

I find the choice of value for "cost-type" to be problematic.  It is a
string in
the base protocol, so changing to an empty object is likely to cause
more issues
than simply omitting it.

Section 4.2 contains mismatched braces/parens for section references.

Section 4.2.2

IMPORTANT: This provides a definition for ReqFilteredCostMap that is
very
different to that in the base specification.  I think that this should
have been
ReqEndpointCostMap.

As before, repeating the definition of EndpointFilter is unnecessary.

Section 4.2.3 - see comment on 4.1.3

Section 5

I would be more comfortable if the examples used obviously-spurious
metrics
(e.g., "cattle-head-count", "smell", "shoe-size", etc...) than these
metrics
that are pretty plausible.  More so when you claim that they are
widely valued,
which implies some sort of validity.

It should be relatively easy to populate Content-Length now that the
examples
are "final".

Section 5.1

You have unmatched braces in "meta" due to the comment.

Section 5.2

Do you want to show one of the examples as having no cost values at
all across
all the cost types?

---

From: "Randriamasy, Sabine (Nokia - FR/Nozay)" <[email-masked]>
Date: Thu, 6 Apr 2017 09:44:36 +0000

Hello Martin,

Thanks a lot for your review. The draft will be updated so as to address them.
Best regards,
Sabine

>>-----Original Message-----
>>From: Martin Thomson [mailto:[email-masked]]
>>Sent: 06 April 2017 04:08
>>To: [email-masked]
>>Cc: [email-masked]; [email-masked]; [email-masked]
>>Subject: Artart telechat review of draft-ietf-alto-multi-cost-08
>>
>>Reviewer: Martin Thomson
>>multiple cost metric instead of a single metric.
>>
>>The document very carefully deals with backwards compatibility with existing
>>ALTO servers and clients.  I don't anticipate many issues arising from the
>>I started reading -07, but finished with -08.  I checked that the issues I raise
>>still exist, but I'm not infallible.  Apologies if I get something wrong.
>>
>>Minor issues: I've identified a few issues that are more than nits, these are
>>The abstract includes considerable justificiation.  An abstract only needs to
>>describe the *what*, not the *why*.  Thus, what is there could be simplified
>>considerably, e.g.,
>>
>>   This document defines a new method for retrieving multiple cost metrics in
>>a
>>   single request for an ALTO filtered cost map.  It also defines improvements
>>The introduction uses a bunch of odd terms.  Some of these are recognizable
>>from the ALTO specification, but some of the jargon seems unnecessary.  In
>>particular, "Internet View", "Provider Network region" and "Vector costs".
>>All of which I think that I understand, but they make the doc hard to follow.
>>
>>Generally, I found the introduction quite hard to follow, both for that reason
>>3. Requesting multiple cost types at the same time is more efficient (for
>>   several reasons).
>>5. Separately, when multiple cost types are present, more sophisticated
>>The example shows an empty cost-type, but the schema you define allows it
>>to be absent.  You REALLY need to pick one.  I don't believe that this is a
>>compatibility issue: once you have determined that a client supports multi-
>>cost, then you can do anything you like, just be clear about it.
>>
>>Section 3.2
>>
>>unconvincing.  However, a new media type that is largely the same as the
>>Just say what it is you expect to happen and don't try to be apologetic about
>>This section is confusing.  You only need to say that you are not altering full
>>cost map resources in any way and that clients need to use filtered cost maps
>>if they want multiple costs at the same time.  (Obviously you could have, but
>>combining many cost types is unwieldy.)  At a minimum, the second
>>paragraph here can be removed.
>>
>>Section 3.6.2
>>
>>IMPORTANT: You don't define what happens when a client provides "or-
>>constraints"
>>and "constraints" at the same time.  There are several valid options, but you
>>need to choose.
>>
>>Section 3.6.3
>>
>>include values from "multi-cost-types", then those types can't be included in
>>Please explain the default value for the index for the "constraints"/"or-
>>constraints" express in this section in addition to where it is hidden in a note
>>later in the document.
>>
>>Section 3.6.5
>>
>>Uppercase for "must not" in the second paragraph.  (The "may" later in the
>>In the example, the resource named "filtered-multicost-map" is provided for
>>legacy reasons only.  Why bother including "max-cost-types" and "cost-type-
>>names" at all when "filtered-cost-map-extended" includes all that and more?
>>
>>Section 4.1.1
>>
>>The definition of the schema here (and later) actually redefines the object
>>completely.  I found that confusing initially.  It would be good to identify the
>>Can testable-cost-type-names be present and empty if cost-constraints is
>>false?
>>I find the redefinition of the optionality of cost-type to be worthy of special
>>note.
>>
>>I find the choice of value for "cost-type" to be problematic.  It is a string in the
>>base protocol, so changing to an empty object is likely to cause more issues
>>than simply omitting it.
>>
>>IMPORTANT: This provides a definition for ReqFilteredCostMap that is very
>>different to that in the base specification.  I think that this should have been
>>ReqEndpointCostMap.
>>
>>metrics (e.g., "cattle-head-count", "smell", "shoe-size", etc...) than these
>>metrics that are pretty plausible.  More so when you claim that they are
>>widely valued, which implies some sort of validity.
>>
>>examples are "final".
>>Do you want to show one of the examples as having no cost values at all
>>across all the cost types?
>>

---

From: "Randriamasy, Sabine (Nokia - FR/Nozay)" <[email-masked]>
Date: Tue, 25 Apr 2017 17:26:40 +0000

Hello Martin, 

First of all thank you for your thorough review and comments. 
A new version has been uploaded to address them and is available at 
https://datatracker.ietf.org/doc/html/draft-ietf-alto-multi-cost-09 
I hope the updates address your questions. Please, let me know otherwise.

Please see also inline for my answers,

Best regards,
Sabine

>>   to the constraints that can be used for filtered cost maps.

[SR     ]  OK. Abstract shortened in that direction
>>6. This document defines how to do that too.
[SR     ] OK, section revised in that direction

>>Network Service Provider, maybe more.  Please check and remove those
[SR     ] OK: removed 3 definitions

>>The RFC 7285 section reference thing is unnecessary.
[SR     ] OK: moved to end of section 2, to avoid repeating "of [RFC7285]" too often.

>>This document doesn't cite RFC 2119, but it uses the keywords.
[SR     ] RFC 2119 is cited on page 1, section " Requirements Language" and section "9.1.  Normative References". Should it be referenced elsewhere? 

>>cost, then you can do anything you like, just be clear about it.
[SR     ] OK, added explanations before and after the example 
>>it.  What you have appears to be a workable design.
[SR     ] OK, removed 1st paragraph.
>>paragraph here can be removed.
[SR     ] OK shortened 2nd paragraph
>>and "constraints" at the same time.  There are several valid options, but you
[SR     ] OK added text at the end on why a client cannot provide both
>>"constraints"/"or-constraints".
[SR     ] OK added this after 1st paragraph
>>constraints" express in this section in addition to where it is hidden in a note
[SR     ] OK, moved text to example in section 5.4
>>paragraph might be better as "can".)
[SR     ] OK, done. Also added an sentence in 1st paragraph to clarify it. 
>>names" at all when "filtered-cost-map-extended" includes all that and more?
[SR     ] OK, before example, added text explaining when it is useful to specify both. 
Actually, "cost-type-names" is always present in filtered cost map capabilities.
>>*changes* from the base specification somehow.
[SR     ] indeed. In RFC7285 {11.3.2.4}, member "cost-constraints" is not optional in object specification but is optional in the member description. Added a sentence at the end of 1st paragraph stressing the presence of the 2 new member listed in the beginning of this paragraph. 
Also added some text in introduction of section 4, on the notations, in particular "<m..n>". 
>>The first part of the definition permits that, the second forbids it.
[SR     ] The first part specifies member "testable-cost-type-names" as having "<1..*>" elements, if present.  
Added item to describe "cost-constraints" and moved text from "testable-cost-type-names" there. 
>>The redefinition of PIDFilter is unnecessary.
[SR     ] OK, deleted
>>IMPORTANT: pids is optional in RFC 7285.  Why the change?
[SR     ] OK, corrected this, pids optional again. Thanks for catching this.
>>I find the redefinition of the optionality of cost-type to be worthy of special
[SR     ] OK, added sentence in definition of "cost-type" member
>>In the definition of "or-constraints", you use a "database query"
[SR     ] OK, replaced "database query" with "words"
>>base protocol, so changing to an empty object is likely to cause more issues
[SR     ] Actually, "cost-type" is defined in {10.7} as an object unlike "cost-type-names" which is array of JSONString.
>>Section 4.2 contains mismatched braces/parens for section references.
[SR     ] OK, corrected
>>different to that in the base specification.  I think that this should have been
[SR     ] OK, corrected this and replaced " ReqFilteredCostMap " with "ReqEndpointCostMap", thanks for catching this.
>>As before, repeating the definition of EndpointFilter is unnecessary.
[SR     ] OK deleted
>>Section 4.2.3 - see comment on 4.1.3
[SR     ] OK, please see my answer.
>>widely valued, which implies some sort of validity.
[SR     ] OK, replaced "hopcount" with "shoesize" and "bandwidthscore" with "sceneryrate" and adapted section introduction accordingly. 
>>It should be relatively easy to populate Content-Length now that the
[SR     ] OK, done
>>You have unmatched braces in "meta" due to the comment.
[SR     ] OK, corrected this. Also removed the comments in the IRD
>>across all the cost types?
[SR     ] OK, updated text to say this only holds between PID2 and PID3

---

From: Martin Thomson <[email-masked]>
Date: Wed, 26 Apr 2017 16:39:07 +1000

> [SR     ] RFC 2119 is cited on page 1, section " Requirements Language" and section "9.1.  Normative References". Should it be referenced elsewhere?

The convention is to put those in the body, I missed it in the boilerplate.

I skimmed the other changes, and they look fine.

---

From: "Randriamasy, Sabine (Nokia - FR/Nozay)" <[email-masked]>
Date: Wed, 26 Apr 2017 07:38:19 +0000

Hello Martin,

Thanks, I'll update with RFC 2119 cited at appropriate places 
Sabine

>>Sent: 26 April 2017 08:39
>>To: Randriamasy, Sabine (Nokia - FR/Nozay) <sabine.randriamasy@nokia-
>>bell-labs.com>
>>Cc: [email-masked]; [email-masked]; [email-masked]; draft-ietf-alto-multi-
>>[email-masked]
>>Subject: Re: Artart telechat review of draft-ietf-alto-multi-cost-08
>>
>>> [SR     ] RFC 2119 is cited on page 1, section " Requirements Language" and
>>section "9.1.  Normative References". Should it be referenced elsewhere?
>>

---

From: "Randriamasy, Sabine (Nokia - FR/Nozay)" <[email-masked]>
Date: Thu, 27 Apr 2017 11:57:22 +0000

Hello Martin,

I just posted an update where the " Requirements Language" text has been moved in a section 1.1.
As I saw it on a number of other ietf drafts, I also added the sentence 
"When the words appear in lower case, their natural language meaning is used."

The update and status are available at https://datatracker.ietf.org/doc/draft-ietf-alto-multi-cost/

Thanks,
Sabine