"Eric Voit (evoit)" <evoit@xxxxxxxxx> wrote: > > From: Martin Bjorklund, March 23, 2018 7:37 AM > > > > 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. > > 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 > > > > > 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 >
