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

[Martin Bjorklund <[e-mail]>]

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.


> > 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".

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
> > >
>