Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clarification for allow_aggregation control #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

clarification for allow_aggregation control #162

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95cf36d
Update gnmi-specification.md
osquitun Mar 8, 2022
3d37134
Merge pull request #1 from osquitun/osquitun-patch-clarification-allo…
osquitun Mar 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions rpc/gnmi/gnmi-specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ For any JSON encoding:
* Where the data item referred to has child nodes, the `val` field contains a
serialised JSON entity (object or array) corresponding to the referenced
item.
* The serialization MUST follow the methods and fields in the
Subscribe RPC in [Section 3.5.1](#351-managing-subscriptions).

Using the following example data tree:

Expand Down Expand Up @@ -1603,10 +1605,10 @@ affecting the sample accuracy and freshness to the client, and as a result, on
the client's ability to react to events on the target.

Since it is not possible for the target to infer whether its clients are
sensitive to the latency introduced by bundling, if a target implements
optimizations such that multiple `Update` messages are bundled together,
it MUST provide an ability to disable this functionality within the
configuration of the gNMI service. Additionally, a target SHOULD provide means
sensitive to the latency introduced by bundling, the target MUST follow the
`SubscriptionList` fields in [Section 3.5.1.2](#3512-the-subscriptionlist-message)
that indicate if `allow_aggregation` is permitted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that maybe we need a larger change to the wording here. I think we should clarify what bundling means here vs. aggregation. Specifically, we were trying to give guidance here as to how many update messages there were per notification.

In aggregation, I think we are looking at how many leaves per update there are. i.e., without allow_aggregation this should be 1.

I'd propose we make this clarification, and then add some text as to when aggregation is triggered. WDYT?

Additionally, a target SHOULD provide means
by which the operator can control the maximum number of updates that are to be
bundled into a single message, This configuration is expected to be implemented
out-of-band to the gNMI protocol itself.
Expand Down