Re: [Netconf] mbj's WGLC review of yang-push-17

[Martin Bjorklund <[e-mail]>]

Hi,


Alexander Clemm <[e-mail]> wrote:
> Hi Martin,
> 
> please see my responses inline, <ALEX>, trimming the closed stuff
> ("Ok.")
> 
> Thanks
> --- Alex
> ...
> > 
> > > > > > > > o  3.11.1
> > > > > > > >
> > > > > > > >   The text says:
> > > > > > > >
> > > > > > > >    It is not
> > > > > > > >    required to merge pending update messages.
> > > > > > > >
> > > > > > > >   This can be read as indicating that a server MAY merge pending
> > > > > > > >   update messages.  I assume that it should say that pending
> > > > > > > >   update
> > > > > > > >   messages MUST NOT be merged.
> > > > > > >
> > > > > > > <ALEX> Hmm.  I am not sure I agree.  The server is not
> > > > > > > required to merge pending update messages - i.e. can send
> > > > > > > multiple messages each with a separate update record.
> > > > > > > However, there is no reason to preclude that they could be
> > > > > > > combined.  So, I don't think an update is needed here.
> > > > > > > </ALEX>
> > > > > >
> > > > > > So you say that the text means that the server MAY merge pending
> > > > > > update messages in this case?  If so, I think you should update
> > > > > > the text so that this is clear.
> > > > > >
> > >
> > > <ALEX3> OK, how about saying "The server MAY (but does not have to)
> > > merge pending update messages". </ALEX3>
> > 
> > Well, the term "update message" is not defined, so it shouldn't be
> > used.
> > Presumably you mean "update record".
> 
> <ALEX> sure </ALEX>
> 
> > 
> > But I still don't really understand how this is supposed to work.  The
> > push-code in
> > the publisher creates an update record A and hands it over to the
> > transport
> > layer, where it is queued:
> > 
> >    It is perfectly acceptable to have a series of "push-change-
> >    update" notifications (and even "push update" notifications) serially
> >    queued at the transport layer awaiting transmission.
> > 
> > Then the push-code in the publisher creates another update record B
> > and hands
> > it over to the transport layer.
> > 
> > Are you saying that the transport layer code now can merge these two
> > update
> > records into a single record?

You didn't answer this question.

> > So if record A had "create foo, delete
> > bar" and
> > record B had "delete foo", there would be a single record with "delete
> > bar,
> > delete foo"?

You didn't answer this question.

> > If two records are merged, what happens to the idea of using the
> > 'patch-id' as
> > an indication of lost records?

You didn't answer this question.


> <ALEX> All this is saying is that it is up to the publisher to decide
> how to "group" the updates: whether to send a single update record (in
> a single notification), or whether to create multiple records (sent
> with multiple notifications).
> To make this clear, slight updates as follows:
> " It is perfectly acceptable to have a series of "push-change-update"
> notifications (and even "push update" notifications) serially queued
> at the transport layer awaiting transmission.  It is not required for
> the publisher to merge pending update records sent at the the same
> time."
> </ALEX>

I think that if you really want to allow such merging, it needs to be
described how it is supposed to be done, e.g. wrt. the patch-id.  But
I would rather see that this "merging" feature is removed.

(In one sense, it doesn't matter much; it is an optional behaviour so
since I don't understand it, I can simply just not implement it.)



/martin