Hi, "Eric Voit (evoit)" <evoit@xxxxxxxxx> wrote: > Hi Martin, > > > From: Martin Bjorklund, March 16, 2018 4:19 AM > > > > Hi, > > > > Andy Bierman <andy@xxxxxxxxxxxxx> 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. 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. 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 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. > > > 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
