openconfig / gnmi

gRPC Network Management Interface
Apache License 2.0
474 stars 197 forks source link

Undocumented use of gNMI Notification field `atomic` #180

Open earies opened 4 months ago

earies commented 4 months ago

A gNMI Notification message contains a boolean field named atomic per:

https://github.com/openconfig/gnmi/blob/master/proto/gnmi/gnmi.proto#L89-L91

  // This notification contains a set of paths that are always updated together
  // referenced by a globally unique prefix.
  bool atomic = 6;

However the comments above are where this definition ends and is left to interpretation. The gNMI specification does not carry any additional definition or desired usage.

Now this could be interpreted a few ways it seems:

  1. Set the value to true for any YANG model structure that carries the oc-ext:telemetry-atomic annotation essentially just reflecting a boolean signal according to the data content
  2. Set the value to true per the comment when a prefix is carried in the Notification message indicating that all Update messages as part of the Notification "conform" to the prefix concatenation

The latter is completely unnecessary as the existence of a prefix is enough to know that all subsequent Update messages packed into that Notification are part of the same prefix

The prior means that there is a direct correspondence to the data-model annotation.

Is this value necessary? How is it intended to be used or of value to a consumer?

Thx

robshakir commented 4 months ago

A schema-unaware collector that receives an update with atomic = true can optimise its storage by not individually caching paths below this prefix. i.e., if /a/b/c is received with atomic = true then the collector can cache values at /a/b/c and return is value rather than storing /a/b/c/d and /a/b/c/e separately.

This is implemented in the collector in openconfig/gnmi.

robshakir commented 4 months ago

The annotation that is used in OpenConfig models is telemetry-atomic.

telemetry-on-change is used to indicate that the data model expects that this value is sent as ON_CHANGE in TARGET_DEFINED mode of gNMI (or any other event-driven update model in a different RPC).

earies commented 4 months ago

The annotation that is used in OpenConfig models is telemetry-atomic.

telemetry-on-change is used to indicate that the data model expects that this value is sent as ON_CHANGE in TARGET_DEFINED mode of gNMI (or any other event-driven update model in a different RPC).

sorry yes - typed one thing... meant the other

earies commented 4 months ago

A schema-unaware collector that receives an update with atomic = true can optimise its storage by not individually caching paths below this prefix. i.e., if /a/b/c is received with atomic = true then the collector can cache values at /a/b/c and return is value rather than storing /a/b/c/d and /a/b/c/e separately.

This is implemented in the collector in openconfig/gnmi.

Would you then agree this goes into bucket (1) above where this is directly correlated to the annotation in the YANG model case (and up to however anyone wishes outside of those bounds)?

e.g.

/system/messages/state/message is meant to be delivered as a single Notification PDU with multiple Update messages comprising the contents of the subtree, the prefix could be any combination leading up to that container and the Notification is set to atomic=true

The collector need not know the YANG model contents but the producer is merely reflecting the intent

If so, I can draft up a PR to tighten this up in the spec + proto IDL

robshakir commented 4 months ago

I'm not entirely sure I understand.

Technically, with atomic = true, I think a target is allowed (based on the current loose spec) to annotate this anywhere, and a collector can decide what to do based on this. It is likely better that this is something that is marked according to the schema, is that your point here?

earies commented 4 months ago

I'm not entirely sure I understand.

Technically, with atomic = true, I think a target is allowed (based on the current loose spec) to annotate this anywhere, and a collector can decide what to do based on this. It is likely better that this is something that is marked according to the schema, is that your point here?

You got it...

For where YANG is used to describe the data content and in the case of OpenConfig modeling, we have a direct correlation to the oc-ext:telemetry-atomic annotation and when this boolean field should be flipped to true - no other expectation otherwise (e.g. this should not be flipped to true for random subtrees that do not carry this extension imo)

If native modeling in an implementation chooses to drive similar behavior, this can be purely internal (less desirable) or driven by way of a schema marking/annotation (more desirable to have a more definite contract)

If you agree - I think some mention of this behavior/expectation is warranted in the gNMI spec (which does not contain anything towards the usage of this field today) and likely comment rewording in the proto IDL

robshakir commented 4 months ago

Makes sense -- happy to have this clarification. Randomly using atomic is indeed undesirable, since the aforementioned optimisation (caching at the prefix that is atomic) can also result in having no means to subtrees of that prefix.