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

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

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

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

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

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. In addition, we might end up sending a stream of information to the subscriber which is not sufficient, and therefore not verifiably relevant.

> > > >    If a "stop-time" parameter is included, it MAY also be earlier than
> > > >    the current time and MUST be later than the "replay-start-time".
> The
> > > >    publisher MUST NOT accept a "replay-start-time" for a future time.
> > > >
> > > >    >> MUST be later (if the start-time) if supplied
> > > >    >> MAY be before current time?  Inconsistent with start-time
> > > >       MUST have events that exist
> > > >    >> MUST NOT accept future start-time different than 5277, but OK
> > > >       because that was a bad requirement
> > >
> > > I have also pointed out in earlier reviews that the requirement that
> > > the replay-start-time cannot be in the future is problematic:
> >
> > Yes.  The thinking both of you passed along drove this modification.
> >
> > >   I know that this text is also present in RFC 5277, but I think it
> > >   needs to be changed.  Which current time?  Probably the server's,
> > >   but how would a client know that?  This is a problem that we faced
> > >   when implementing 5277.   I think we should remove this
> > >   requirement, since it doesn't add any value anyway.
> > >
> > > IMO, if the server gets a replay-start-time that is later than the
> > > latest stored notif, it will send a <replay-completed> notif, and
> > > then move on.
> > > This is a
> > > very simple behavior that doesn't rely on synchronized clocks or
> > > anything like that.
> >
> > The text which I suggested back to Andy :
> >
> >    If the "replay-start-time" is later the scope of time covered by the
> >    replay buffer, then the publisher MUST send a "replay-completed"
> >    notification immediately after the after a successful establish-
> >    subscription RPC response.
> 
> Good.
> 
> > > [...]
> > >
> > > > 2.5.2.  Creating a Configured Subscription
> > > >
> > > >        In this case, when there is
> > > >    something to transport for an active subscription, transport specific
> > > >    call-home operations will be used to establish the connection.
> > > > When
> > > >
> > > >    >> is this normative or is callhome optional-to-implement?
> > > >
> > > >    With active configured subscriptions, it is allowable to buffer event
> > > >    records even after a "subscription-started" has been sent.  However
> > > >    if events are lost (rather than just delayed) due to replay buffer
> > > >    overflow, a new "subscription-started" must be sent.  This new
> > > >    "subscription-started" indicates an event record discontinuity.
> > > >
> > > >   >> this is confusing to send multiple "subscription-started" events.
> > > >
> > > >    To see an example at subscription creation using configuration
> > > >    operations over NETCONF, see Appendix A of
> > > >    [I-D.draft-ietf-netconf-netconf-event-notifications].
> > > >
> > > >    >> IMO the examples should be moved to this draft
> > >
> > > +1
> >
> > But those examples are transport specific.  This will make the
> > document less applicable for other transports.
> 
> I don't think it will.  Just write that the example is using NETCONF XML or
> RESTCONF JSON or whatever.  Compare with the RESTCONF draft, it has
> plenty of examples despite the fact that it supports two encodings.
> 
> I think having the examples in this draft is the right thing to do.

For the RESTCONF draft, there isn't another natural matching transport document where the information naturally fits.  And by doing the split in this way means we can point to a fairly extensive list of examples in one place which is transport-relevant.

Eric

> > > > 2.7.1.  subscription-started
> > > > 2.7.2.  subscription-modified
> > > >
> > > >   >> what is the value of returning all the input or configuration
> > > >     parameters in these notifications?  For a dynamic subscription,
> > > >     the only receiver just sent that info and does not need it.
> > > >     For a configured subscription, that data can be read from
> > > >     the configuration datastore.
> > >
> > > I agree.  Removing these redundant parameters would also simplify
> > > the models quite a bit.
> >
> > I replied on the thinking here to Andy.
> >
> > Basically it is possible that you could only send the contents of the
> > leafref for dynamic subscriptions.  However this introduces
> > complexity, as should have a notification type and set a different
> > expectation of what would be populated with a dynamic subscription.
> > So in the end, we can do it.  But it makes the model more complicated
> > (although the tree gets smaller.)
> >
> > Also this assumes that the receiver can do a read.  (For IoT, this
> > might not be the case.)
> >
> > Beyond that, if the parameters change multiple times on a configured
> > subscription, you might not be quick enough to do a read in time to
> > know the parameters during a transient period.
> >
> > Finally, a configured receiver might have lost state, so why not
> > refresh the full set?  There is little cost to refreshing the full
> > view of the subscription.
> >
> > So in the end, this a complex simplification drives error cases and
> > more variations to process for the receiver.
> >
> > > [...]
> > >
> > > > 4A) message encoding
> > > >
> > > >   feature encode-json {
> > > >     description
> > > >       "This feature indicates that JSON encoding of notification
> > > >        messages is supported.";
> > > >   }
> > > >
> > > >   feature encode-xml {
> > > >     description
> > > >       "This feature indicates that XML encoding of notification
> > > >        messages is supported.";
> > > >   }
> > > >
> > > >   identity encodings {
> > > >     description
> > > >       "Base identity to represent data encodings";
> > > >   }
> > > >
> > > >   identity encode-xml {
> > > >     base encodings;
> > > >     if-feature "encode-xml";
> > > >     description
> > > >       "Encode data using XML";
> > > >   }
> > > >
> > > >   identity encode-json {
> > > >     base encodings;
> > > >     if-feature "encode-json";
> > > >     description
> > > >       "Encode data using JSON";
> > > >   }
> > > >
> > > >   typedef encoding {
> > > >     type identityref {
> > > >       base encodings;
> > > >     }
> > > >     description
> > > >       "Specifies a data encoding, e.g. for a data subscription.";
> > > >   }
> > > >
> > > >     leaf encoding {
> > > >       type encoding;
> > > >       mandatory true;
> > > >       description
> > > >         "The type of encoding for the subscribed data.";
> > > >     }
> > > >
> > > >   >> IMO all YANG definitions related to message encoding should
> > > >      be removed because they are in conflict with existing protocols.
> > > >      NETCONF defines XML encoding. HTTP already defines
> > > >      media type handling for message encoding (Accept, Content-Type)
> > > >      There is no definition how to use JSON with NETCONF.
> > >
> > > +1
> >
> > Per response to Andy:
> >
> > It is true that it is possible to populate unsupported mixtures of
> > protocol and encoding.  However:
> > (a) for configured subscriptions, we must be able to select different
> > encodings for a single type of transport
> > (b) checking what is an invalid/unsupported combination for a platform
> > is quite easy
> >
> > While it is possible to build a structure which enforces valid
> > combinations with YANG, this would add complexity, especially as
> > vendor custom encodings will also become new identities under the base
> > encoding.  If there is some YANG structure which exists for such
> > enforcement of protocol and encoding (which would be something likely
> > common with other solutions), do you have a link?
> 
> I replied to this issue in the other thread, so let's continue the discussion
> there.
> 
> 
> /martin