Re: [i2rs] AD review of draft-ietf-i2rs-ephemeral-state-18 - REQ-12

[Joe Clarke <[e-mail]>]

On 10/5/16 09:30, Susan Hares wrote:
> Joel:
>
> WFM - see inclusion in version 19.

Agreed.  And I agree with Alia's comment.  The overwriting client should 
just succeed.  Why let them know (again) that it worked via a notification.

Joe

>
> Sue
>
> -----Original Message-----
> From: Joel M. Halpern [mailto:[e-mail]]
> Sent: Wednesday, October 5, 2016 3:26 AM
> To: Alia Atlas; [e-mail]; [e-mail]
> Subject: Re: [i2rs] AD review of draft-ietf-i2rs-ephemeral-state-18 - REQ-12
>
> We probably should tweak the wording on REQ-12.  The notification is only
> needed when the new operation succeeds.
> When the new operation fails, the requester will receive an error, and the
> original state is still there, so no notification is needed.  I should have
> realized that in my earlier review.
>
> Suggested fix, add text at left margin:
>     Ephemeral-REQ-12: When a collision occurs as two clients are trying
>     to write the same data node, this collision is considered an error
>     and priorities were created to give a deterministic result.  When
>     there is a collision,
> and the data node is changed,
>        a notification (which includes indicating data
>     node the collision occurred on) MUST BE sent to the original client
>     to give the original client a chance to deal with the issues
>     surrounding the collision.  The original client may need to fix their
>     state.
>
> Yours,
> Joel
>
> On 10/4/16 10:37 PM, Alia Atlas wrote:
>> Hi,
>>
>> As is customary, I have done my AD review of
>> draft-ietf-i2rs-ephemeral-state-18.  First, I would like to thank Sue
>> and Jeff for their hard work pulling this document together in an
>> effort to add clarity to the requirements.
>>
>> I do have a number of comments - many relatively minor.  Assuming a
>> fast turn-around, as usual from I2RS, we should be able to have this
>> on the Oct 27 telechat - which will mean it needs to enter IETF Last
>> Call before the end of this week.
>>
>> Here is my review:
>>
>> Major:
>>
>> 1) Ephemeral-REQ-12:  This specifies that a notification be sent to
>> the original client, regardless of whether it won or lost the priority
>> collision.
>> I had assumed that the notification would go to either the requesting
>> client or the original client depending on which one lost the priority
> comparison.
>> I have some concerns about an indirect flood of notifications caused
>> by a requesting client that has the lower priority.  Regardless,
>> clarifying that the lower-priority client is notified is important.
>>
>>
>>
>> Minor:
>> a) Intro: Remove "3 suggest protocol strawman" as something that
>>    the I2RS requirements must do.  I know that is how the process
>>    has been working out - but it isn't dictated by the technology
>>    at all - as the other 2 are.  Similarly, replace the following
>>    paragraph "The purpose of these requirements and the suggested
>>    protocol strawman is to provide a quick turnaround on creating
>>    the I2RS protocol." with something like "The purpose of these
>>    requirements is to ensure clarity during I2RS protocol creation."
>>
>> b) Section 2:  "The following are ten requirements that [RFC7921]
>>    contains which provide context for the ephemeral data state
>>    requirements given in sections 3-8:"
>>       How about "The following are requirements distilled from [RFC7921]
>>      that provide context for..."
>>
>>     1)  Not relevant for ephemeral - this matters for pub/sub (suggest
>> removal)
>>     2)  Relevant for ephemeral b/c of vague performance requirements on
>>         possible solutions.
>>     3)  What changes if the data model is protocol dependent?  Is this
>> just that
>>         the model may be an augmentation/extension of an existing module?
>>     4)  Absolutely - keep
>>     5)  Absolutely - keep
>>     6)  Remove - not relevant for ephemeral just security requirements
>>     7)  Remove - not relevant for ephemeral just security requirements
>>     8)  Absolutely - keep (but says storing secondary identity on
>> deletion when
>>         that isn't mentioned for (4) b/c it's about priority - so
>> clarify slightly)
>>     9)  Absolutely - keep
>>    10)  Remove - not relevant for ephemeral
>>
>> c) Sec 3.3 bullet 2:  This refers to YANG data model instead of YANG
>> module as
>>    in bullet 1.  If there's a reason for the difference, please
>> clarify and otherwise
>>    make consistent.
>>
>> d) Sec 5 & 6 for NETCONF and RESTCONF are the same requirements.
>> Please consolidate into a section of "The changes to NETCONF and the
>> conceptual changes to RESTCONF are"
>>
>> e) Sec 8:  I found this pull-out unclear.  "multiple operations in one
>>       or more messages; though errors in
>>       message or operation will have no effect on other messages or
>>       commands even they are related."
>>
>>      I think you mean "Multiple operations in one message can be sent.
>> However
>>      an error in one operation MUST NOT stop additional operations
>> from being
>>      carried out nor can it cause previous operations in the same message
> to
>>      be rolled back."
>>
>> Nits:
>>
>> i) Abstract: "attempting to meet I2RS needs has to provide"/
>> "attempting to meet the needs of I2RS has to provide"
>>
>> ii) 3.2: "MPLS LSP-ID or BGP IN-RIB"  please expand acronyms
>>
>> iii) Sec 5 last sentence:  Either missing a ( or has an unneeded ).
>>
>> iv) Ephemeral-REQ-11:  "I2RS Protocol I2RS Protocol" repeated
>>
>>
>>
>> _______________________________________________
>> i2rs mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/i2rs
>>
>
> _______________________________________________
> i2rs mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/i2rs
>