What should a gNMI server return when a default value is queried?

wenovus commented 3 years ago

There are four independent cases considered below. All examples use the /interfaces/interface/state/enabled leaf, which has a default value of true.

Question

For all queries below, consider the case that the interface at /interfaces/interface[name="foo"] exists, and that its "enabled" field has its default value of true.

The question for all of them is, must ("/interfaces/interface[name="foo"]/state/enabled", true) be returned by GET?

1 Non-wildcard leaf query

e.g. GET /interfaces/interface[name="foo"]/state/enabled

2 Non-wildcard non-leaf query

e.g. GET /interfaces/interface[name="foo"]

3 Wildcard leaf query

e.g. GET /interfaces/interface[name="*"]/state/enabled

4 Wildcard non-leaf query

e.g. GET /interfaces/interface[name="*"]

wenovus commented 3 years ago

@dan-lepage pointed out the interesting case where /interfaces/interface[name="foo"] does not exist. According to the spec, GET should also return nothing since it's a valid path. So, if GET on /interfaces/interface[name="foo"]/state/enabled doesn't return the default value, then it would be impossible to distinguish between a value existing as its default value, and a value that doesn't exist due to one of its an ancestor's list element not existing.

My current preference: By requiring GET to always return a value for a leaf GET/SUBSCRIBE if it exists, even if it's the default value, it removes this ambiguity.

This is also relevant for non-leafs. While I'm guessing that list keys should not have default values specified, it's possible to have containers underneath lists that make it possible for the same ambiguity to occur. Although this case may occur less often, we may still need to consider it for completeness.

Related question: Are we planning on supporting presence-containers?

dan-lepage commented 3 years ago

According to the spec, GET should also return nothing since it's a valid path.

If I'm reading the spec correctly, it doesn't actually say what should happen on a GET. On a SUBSCRIBE with ONCE set, it should actually never return - "In the case that a particular path does not (yet) exist, the target MUST NOT close the RPC, and instead should continue to monitor for the existence of the path, and transmit telemetry updates should it exist in the future."

How does a GET return "nothing" for a syntactically valid but nonexistent path? E.g. does GET on /interfaces/interface[name="NotAnInterface"] simple not contain any updates in the response, or does it return a default "empty" value of either a JSON {} or some special null value?

wenovus commented 3 years ago

Yes I mixed them up, I agree with you on both GET and SUBSCRIBE. I'm now interpreting the spec to say that SUBSCRIBE[ONCE] would hang if the path doesn't exist.

gcsl commented 3 years ago

Subscribe[once] should never "hang". It should return only what it has "now". The wording around not closing the RPC is specific to the streaming such that a missing path should never be a hard error which enables watching for something you expect to appear.

On Thu, Sep 2, 2021 at 4:05 PM Wen Bo Li @.***> wrote:

Yes I mixed them up, I agree with you on both GET and SUBSCRIBE. I'm now interpreting the spec to say that SUBSCRIBE[ONCE] would hang if the path doesn't exist.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/142#issuecomment-912015151, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEL4QL47Q7GNQLCDGVRHU5DT77KIHANCNFSM5BDIAKOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

gcsl commented 3 years ago

On the Get, the response should be empty, no updates, not a value indicating empty.

On Fri, Sep 10, 2021 at 1:36 PM Carl Lebsack @.***> wrote:

Subscribe[once] should never "hang". It should return only what it has "now". The wording around not closing the RPC is specific to the streaming such that a missing path should never be a hard error which enables watching for something you expect to appear.

On Thu, Sep 2, 2021 at 4:05 PM Wen Bo Li @.***> wrote:

Yes I mixed them up, I agree with you on both GET and SUBSCRIBE. I'm now interpreting the spec to say that SUBSCRIBE[ONCE] would hang if the path doesn't exist.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/142#issuecomment-912015151, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEL4QL47Q7GNQLCDGVRHU5DT77KIHANCNFSM5BDIAKOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

wenovus commented 3 years ago

Ok so the wording around closing the RPC in section 3.5.1.3 only applies to 3.5.1.5.2 (STREAM), but not 3.5.1.5.1 (ONCE)? If so I think the spec should be edited to make this clear.

wenovus commented 3 years ago

Suggested change to 3.5.1.3

I wrapped added parts with @@

There is no requirement that the path specified in the message must exist within
the current data tree on the server. While the path within the subscription
SHOULD be a valid path within the set of schema modules that the target
supports, subscribing to any syntactically valid path within such modules MUST
be allowed. In the case that a particular path does not (yet) exist, the target
MUST NOT close the RPC, and instead should continue to monitor for the existence
of the path, and transmit telemetry updates should it exist in the future.

There is no requirement that the path specified in the message must exist within
the current data tree on the server. While the path within the subscription
SHOULD be a valid path within the set of schema modules that the target
supports, subscribing to any syntactically valid path within such modules MUST
be allowed. In the case that a particular path does not (yet) exist, @@ the target would send just a sync_response for ONCE and POLL subscriptions, but
MUST NOT close the RPC for STREAM subscriptions @@, and instead should continue to monitor for the existence
of the path, and transmit telemetry updates should it exist in the future.

gcsl commented 3 years ago

This SGTM

On Fri, Sep 10, 2021 at 3:01 PM Wen Bo Li @.***> wrote:

Suggested change to 3.5.1.3

I wrapped added parts with @@

There is no requirement that the path specified in the message must exist within the current data tree on the server. While the path within the subscription SHOULD be a valid path within the set of schema modules that the target supports, subscribing to any syntactically valid path within such modules MUST be allowed. In the case that a particular path does not (yet) exist, the target MUST NOT close the RPC, and instead should continue to monitor for the existence of the path, and transmit telemetry updates should it exist in the future.

There is no requirement that the path specified in the message must exist within the current data tree on the server. While the path within the subscription SHOULD be a valid path within the set of schema modules that the target supports, subscribing to any syntactically valid path within such modules MUST be allowed. In the case that a particular path does not (yet) exist, @@ the target would send just a sync_response for ONCE and POLL subscriptions, but MUST NOT close the RPC for STREAM subscriptions @@, and instead should continue to monitor for the existence of the path, and transmit telemetry updates should it exist in the future.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/openconfig/reference/issues/142#issuecomment-917139680, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEL4QL4ANAJKLMRBA6R4QGDUBJIYDANCNFSM5BDIAKOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

wenovus commented 2 years ago

@robshakir Do you agree that a device should send the default value to the user as-if it's a leaf without a default, and set to that value?

liulk commented 2 years ago

If I may pitch in, perhaps config leaves could be allowed either nil or the actual default value, but I feel that the state leaves should always return the effective value. That seems to be consistent with the separation of config and state containers, where admin settings could differ from the operational state.

dplore commented 2 years ago

This is a relatively complex issue. I suggest we break this up into several smaller issues which we can perhaps solve independently.

For this issue let's focus on the SUBSCRIBE behavior. Here's some suggested wording:

SUBSCRIBE STREAM scenarios
- return a value if the value is set or if default data is present
- return an error if the path is syntactically incorrect , close the RPC
- return not supported if the path is syntactically valid but is not implemented and close the RPC
- In the case that a particular syntactically correct path does not (yet) exist
  - @@ the target would send just a sync_response for ONCE and POLL subscriptions,
  - Keep the RPC open for STREAM subscriptions
  - Monitor for the existence of the path, and transmit telemetry updates should it exist in the future.

I will open additional issues for GET regarding default values and GET for a subtree

dan-lepage commented 2 years ago

I commented similarly on #155 - the spec right now says that if you REPLACE a node and don't specify a value for a child, that child should be set to its default value. So I would argue that this isn't two separate issues for subscribe and get, but rather an issue with REPLACE, DELETE, and the initial value of the OpenConfig tree: if we have DELETE behave like an empty REPLACE, then it should be impossible for config or state values that have defaults to ever be unset, because any attempt to erase their value simply sets them to their default values.

wenovus commented 2 years ago

I agree that the question here is about what should be returned for a default-valued leaf. Since both GET and SUBSCRIBE responses use gnmipb.Notification as the response type, it seems to me that their behaviours should be the same.

dan-lepage commented 2 years ago

I agree that the question here is about what should be returned for a default-valued leaf.

I'm actually arguing that we should address this even earlier, at Replace/Delete time - if a user tries to unset a leaf, and that leaf has a default value, the server should treat this as the user updating that leaf to its default value. Then you don't need any special cases for GET or SUBSCRIBE - as far as they're concerned, a default leaf is no different from any other leaf.

wenovus commented 2 years ago

I said in https://github.com/openconfig/reference/issues/142#issuecomment-1050233266

Since both GET and SUBSCRIBE responses use gnmipb.Notification as the response type, it seems to me that their behaviours should be the same.

However, I also see in https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#332-the-getresponse-message

The target MUST generate a Notification message for each path specified in the client's GetRequest

Therefore, there indeed could be differences between Subscribe and Get. We should clarify if and how they might be different.

DanG100 commented 2 years ago

In a streaming subscription What does a delete look like on a leaf with a default value?

Should the server send an Update setting the leaf back to its default value?
Should the server send a Delete for the leaf and the client is responsible for knowing what the default value should be?
Should the server send a Delete and Update in the same Notification for the leaf?

gcsl commented 2 years ago

Can we include a concrete example of such a leaf for discussion? I feel like this is getting into the hypothetical weeds. From a telemetry perspective a leaf either exists or it doesn't.

dan-lepage commented 2 years ago

A concrete example would be if you issue a DELETE for /interfaces/interface[name="foo"]/config/enabled. The default value for this path is true, so I'd say the server should respond exactly as though you'd replaced it with the default value. In this case, the streaming subscription to /interfaces/interface[name="foo"]/state/enabled would receive an Update setting the value back to true.

robshakir commented 2 years ago

there are two options, as I see it here:

the target sends a delete (proto) for the path
the target sends an update with the new value being set to the default.

I think both are semantically equivalent, a particular leaf being NotFound in the tree means that it is running as its default. I'm not sure it's actually worth being prescriptive here -- client implementations that are dealing with schemas that have defaults MUST handle both of these cases IMHO (and hence, we do in ygot with a GetXXX method).

I have a mild preference for the first here, because it means that the tree size is minimised -- i.e., we do not need to store things in the telemetry collector that have defaults just because they have defaults.

dplore commented 2 years ago

I think 2. may be more compliant with https://datatracker.ietf.org/doc/html/rfc6020#section-7.6.1. Thoughts?

   When the default value is in use, the server MUST operationally
   behave as if the leaf was present in the data tree with the default
   value as its value.

   If a leaf has a "default" statement, the leaf's default value is the
   value of the "default" statement.  Otherwise, if the leaf's type has
   a default value, and the leaf is not mandatory, then the leaf's
   default value is the type's default value.  In all other cases, the
   leaf does not have a default value.

robshakir commented 2 years ago

you're right, I think that makes sense.

we should add specification clarifications, as well as tests for these decisions.

openconfig / reference