Re: [yang-doctors] [Netconf] Yangdoctors last call review of draft-ietf-netconf-subscribed-notifications-10

["Eric Voit (evoit)" <[e-mail]>]

Hi Martin,

> Hi,
> 
> "Eric Voit (evoit)" <[e-mail]> wrote:
> > > From: Martin Bjorklund, April 6, 2018 10:31 AM
> > >
> > > Hi,
> > >
> > > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > Hi Martin,
> > > >
> > > > After reflecting on this for a bit, I am ok with your argument
> > > > that if the system is quickly generating notification-messages,
> > > > then the replay buffer might be continuously emptying.  And
> > > > therefore a hint for replay start time might not be accurate for
> > > > very long.  As a result, it is better to start the replay
> > > > immediately with the oldest buffered event.
> > > >
> > > > I have updated the establish-subscription RPC as follows.  Does
> > > > this cover your concern (per this sub-thread)?
> > > >
> > > >   rpc establish-subscription {
> > > >     input {     }
> > > >     output {
> > > >       leaf identifier {      }
> > > >       leaf replay-start-time {
> > > >         if-feature "replay";
> > > >         type yang:date-and-time;
> > > >           description
> > > >             "If a replay has been requested, this represents the
> > > >             earliest time covered by the event buffer for the requested
> > > >             stream.  The value of this object is the
> > > >             'replay-log-aged-time' if it exists.  Otherwise it is the
> > > >             'replay-log-creation-time'.  All buffered event records
> > > >             after this time will be replayed to a receiver.  Note that
> > > >             this object will only be sent if the 'replay-start-time' is
> > > >             later than the time requested by the subscriber.";
> > >
> > > I am ok with this.   But the output leaf should probably not have the
> > > same name as the input leaf; it is not clear which one you refer to
> > > in the last sentence in the description.
> >
> > Done.  It is now:
> >
> >   rpc establish-subscription {...
> >     output {
> >       leaf identifier {      }
> >       leaf replay-start-time-revision {
> >         if-feature "replay";
> >         type yang:date-and-time;
> >           description
> >             "If a replay has been requested, this represents the
> >             earliest time covered by the event buffer for the requested
> >             stream.  The value of this object is the
> >             'replay-log-aged-time' if it exists.  Otherwise it is the
> >             'replay-log-creation-time'.  All buffered event records
> >             after this time will be replayed to a receiver.
> >             This object will only be sent if the start time has been
> >             revised to be later than the time requested by the
> >             subscriber.";
> >       }
> >     }
> >   }
> >
> > >  Also, I suggest you do:
> > >
> > >   s/Note that this/This/
> > >
> > > in the last sentence.
> >
> > Done
> 
> I noted that this change did not make it into -11.

It is in the next version.  You can see it here:
https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-12.txt 

> > > Reading the definition of 'replay-log-aged-time' again, it says:
> > >
> > >    The timestamp of the last event record aged out of the log.
> > >
> > > The document doesn't define what "the timestamp" of an event record
> > > is.
> > > Is this the time that is sent in the "eventTime" field in the
> > > <notification>?   I think this needs to be clarified.
> >
> > How about:
> >
> >       leaf replay-log-aged-time {
> >         if-feature "replay";
> >         type yang:date-and-time;
> >         description
> >           "The timestamp associated with last event record which has
> >            been aged out of the log.  This timestamp identifies how far
> >            back into history this replay log extends, if it doesn't
> >            extend back to the 'replay-log-creation-time'.  This object
> >            MUST be present if replay is supported and any event records
> >            have been aged out of the log.";
> >       }
> >
> >
> > To complement this description, in the "Event Record Delivery"
> > section, I placed the following sentence after the example RFC 5277
> > notification:
> >
> > "In the figure above, the "eventTime" is populated with a timestamp
> > matching the time an originating process identified as when this event
> > occurred."
> 
> I think you need to explain this in more general terms than just in an
> example of a 5277 notification.  Specifically, I think you should introduce a
> generic term for when an event record is generated (maybe "event creation
> time", or even "event timestamp"), and then use this term consistently, and
> also explain in section 2.6 that the 5277 "eventTime" element contains the
> "event timestamp".

Added the term:
Event occurrence time: a timestamp matching the time an originating process identified as when an event happened.
In the Terminology section.

In the "Event Record Delivery" Section, the text before the example now says:
This notification message MUST be encoded in a <notification> message as defined within [RFC5277], Section 4. And per [RFC5277]'s "eventTime" object definition, the "eventTime" is populated with the event occurrence time.

Updated subscribed-notification document can be seen at:
https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netconf-subscribed-notifications-12.txt 

Also imported the "event occurrence time" text into the notification-messages draft.

Eric

> Then this term should be imported by
> draft-ietf-netconf-notification-messages and used there as well.
> 
> 
> /martin
> 
> 
> >
> > Eric
> >
> > > /martin
> > >
> > >
> > >
> > > >       }
> > > >     }
> > > >   }
> > > >
> > > > If you are ok, I will spread the change through the rest of the
> > > > document.
> > > >
> > > > Eric
> > > >
> > > >
> > > > > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > > > > From: Martin Bjorklund, March 23, 2018 7:37 AM
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > >
> > > > > > > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > > > > > Hi Martin,
> > > > > > > >
> > > > > > > > > From: Martin Bjorklund, March 16, 2018 4:19 AM
> > > > > > > > >
> > > > > > > > > Hi,
> > > > > > > > >
> > > > > > > > > Andy Bierman <[e-mail]> wrote:
> > > > > > > > >
> > > > > > > > > [...]
> > > > > > > > >
> > > > > > > > > > 2.4.2.1.  Replay Subscription
> > > > > > > > > >
> > > > > > > > > >    If the "replay-start-
> > > > > > > > > >    time" contains a value that is earlier than content stored
> > > > > > > > > >    within
> > > > > > > > > >    the
> > > > > > > > > >    publisher's replay buffer, then the subscription
> > > > > > > > > > MUST be
> > > > > rejected,
> > > > > > > > > >    and the leaf "replay-start-time-hint" MUST be set
> > > > > > > > > > in the
> > > reply.
> > > > > > > > > >
> > > > > > > > > >    >> this is a significant and bad change from RFC
> > > > > > > > > > 5277
> > > behavior
> > > > > > > > > >    >> the start-time says "send all events that you have
> > > > > > > > > >    >> stored
> > > > > > > > > >       since this time" The server sends its oldest event and
> > > > > > > > > >       does
> > > > > > > > > >       not reject the request.  This draft incorrectly
> > > > > > > > > >       interprets
> > > > > > > > > >       the request as "the server MUST have an event
> > > > > > > > > > stored at
> > > least
> > > > > > > > > >       this old"
> > > > > > > > >
> > > > > > > > > I agree, and I have pointed this out in earlier reviews.
> > > > > > > >
> > > > > > > > In our past discussions, it looked like you were ok after
> > > > > > > > reading Yves requirement here:
> > > > > > > >
> > > > > > > > https://www.ietf.org/mail-
> > > > > archive/web/netconf/current/msg12154.htm
> > > > > > > > l
> > > > > > > >
> > > > > > > > Beyond this functional requirement, the design pattern
> > > > > > > > used is that an establish-subscription RPC must send the
> > > > > > > > exact parameters accepted by the publisher.
> > > > > > > >
> > > > > > > >
> > > > > > > > > If a client sends a too early time, and the server
> > > > > > > > > doesn't send the optional hint, the client will have to
> > > > > > > > > guess the time.  Not very robust.
> > > > > > > > >
> > > > > > > > > If the motivation is that the client should be informed
> > > > > > > > > that he might have missed some notifs b/c the
> > > > > > > > > replay-start-time is too early, this information can be
> > > > > > > > > passed in the rpc-reply from establish-subscription instead.
> > > > > > > >
> > > > > > > > Yes this could be done.  But this doesn't follow the
> > > > > > > > design pattern of making the client explicitly ask for what they
> want.
> > > > > > > > Consistent design patterns do matter.
> > > > > > >
> > > > > > > Well, "what they want" depends on the semantics of this leaf!
> > > > > > > If we keep the old semantics, then if the client passes this
> > > > > > > parameter with some start time, it "explicitly asked for
> > > > > > > what it
> > > wanted".
> > > > > > >
> > > > > > > Anyway, if the objective is to ensure that no notifs are
> > > > > > > sent unless the replay-start-time exactly matches the
> > > > > > > event-time of a notification in the buffer, then we can add
> > > > > > > a parameter to ensure that.
> > > > > >
> > > > > > The current definition of replay-start-time is:
> > > > > >
> > > > > > "Used to trigger the replay feature and indicate that the
> > > > > > replay should start at the time specified..."
> > > > > >
> > > > > > To me, that means the replay-start-time has to be covered by
> > > > > > the scope of the replay buffer.  It does not mean that it is
> > > > > > required that the requested replay-start-time needs to exactly
> > > > > > match the time of a buffered event.
> > > > >
> > > > > The text is quite unclear:
> > > > >
> > > > >   Used to trigger the replay feature and indicate that the
> > > > >   replay should start at the time specified.
> > > > >
> > > > > and:
> > > > >
> > > > >   If the "replay-start- time" contains a value that is earlier than
> > > > >   content stored within the publisher's replay buffer, then the
> > > > >   subscription MUST be rejected
> > > > >
> > > > > In lack of a clear definition, I assume that "content stored
> > > > > [in] replay buffer"
> > > > > refers to event records, since I assume that nothing else can be
> > > > > stored in the replay buffer?
> > > > >
> > > > > Next question is what it means that a time value is earlier than
> > > > > "content"?
> > > > > Again, my assumption is that it means "earlier than the 'eventTime'
> > > > > of the event records".  Is this not what is intended?
> > > > >
> > > > > From what you write here though, I think that what you propose
> > > > > is
> > > > > that:
> > > > >
> > > > >   If "replay-start-time" is less than the latest of
> > > > >   "replay-log-aged-time" and "replay-log-creation-time", then the
> > > > >   request is rejected.
> > > > >
> > > > > This must be clarified.  Also, ensure that the required behavior
> > > > > is clearly defined in the YANG module, and not just in the text
> > > > > in the document.
> > > > >
> > > > > But I still think that there should be some way for the client
> > > > > to get all buffered event records, just like what was supported
> > > > > in RFC 5277, without extra round trips.  Note that if the system
> > > > > is quickly generating notifs, the client might need many round
> > > > > trips before it manages to replay anything.
> > > > >
> > > > >
> > > > > > > In all cases, if the client receives a notif with a time
> > > > > > > later than what it asked for, it knows that it might have
> > > > > > > lost some notifs.
> > > > > >
> > > > > > Why would this mean it might have lost some notifs?  In the
> > > > > > current embodiment, the replay will not start unless the
> > > > > > subscriber asked for a time that is within the scope covered
> > > > > > by the buffer.  I.e., a time later than both "replay-log-creation-
> time"
> > > > > > and
> > > "replay-log-aged-time".
> > > > >
> > > > > See above.  But the reason for rejection is that the client
> > > > > might have lost some notifs.
> > > > >
> > > > > > >   leaf replay-exact-start-time {
> > > > > > >     if-feature "replay";
> > > > > > >     when "../replay-start-time";
> > > > > > >     type empty;
> > > > > > >     description
> > > > > > >       "If this parameter is present, and the server does not have
> > > > > > >       any
> > > > > > >        stored event record with 'eventTime' equal to the requested
> > > > > > >        'replay-start-time', then the server MUST reject the
> > > > > > >        request.";
> > > > > > >   }
> > > > >
> > > > > If we add something like this, the leaf name and description
> > > > > text needs to be tweaked for the clarified semantics of
> > > > > replay-start-time.
> > > > >
> > > > > > Something like this parameter *might* be applicable if we
> > > > > > choose to respond to a dynamic replay request with events
> > > > > > later than those requested.  (i.e., in the
> > > > > > establish-subscription success
> > > > > > response.) As noted in other threads, this is a legitimate way
> > > > > > to approach the issue.  However if the WG chooses this way,
> > > > > > this will result in an exception to the design pattern of
> > > > > > requiring the subscriber to ask for what they are going to receive.
> > > > >
> > > > > I disagree.  The client explicitly asks the server to send all
> > > > > buffered event records.
> > > > >
> > > > > > In addition, we might end up sending a stream of information
> > > > > > to the subscriber which is not sufficient, and therefore not
> > > > > > verifiably relevant.
> > > > >
> > > > > It is up to the client to define what is relevant.  Maybe I just
> > > > > want to view the replay buffer for trouble shooting purposes.
> > > > >
> > > > >
> > > > >
> > > > > /martin
> > > >
> >