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.

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

Add UpdatesOnly query option to fetch data for only changed keys #22108

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

tanmayghai18
Copy link

Description

As mentioned long ago (and roughly outlined) in #2791, a limitation on blocking queries is that all K/V entries matching that prefix are returned, even though only 1 key may have changed. This diff adds a new UpdatesOnly option, that when set, introduces server-side filtering to remove all keys/entries (from a Keys() or List() query) that have an old ModifyIndex.

It uses MinQueryIndex as the threshold here, which follows the effective serialization/deserialization chain: WaitIndex --> index --> MinQueryIndex.

  • WaitIndex being the client-side parameter, determining the index the "client" waits for on a blocking query
  • index being the serialized HTTP query parameter representing the WaitIndex
  • MinQueryIndex being the server-side struct (deserialized result of index)

While we (potentially?) wait for the streaming backend: https://developer.hashicorp.com/consul/api-docs/features/blocking#streaming-backend to be implemented for KV endpoints, this can be a good stop-gap solution to enable in-memory, incremental scans or queries over a prefix.

Testing & Reproduction steps

  • Updated unit tests to default minQueryIndex passed down the state layer to be 0 (no filtering)
  • Added a test in state/kvs_test.go to illustrate the filtering based on ModifyIndex

Links

Original GH issue: #2791

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

Copy link

hashicorp-cla-app bot commented Jan 28, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the theme/api Relating to the HTTP API interface label Jan 28, 2025
@tanmayghai18 tanmayghai18 force-pushed the tanmayghai18/add-option-to-only-watch-changed-keys branch from e9953d7 to 87a7836 Compare January 28, 2025 22:22
As mentioned long ago (and roughly outlined) in hashicorp#2791, a limitation on blocking queries is that all K/V entries matching that prefix are returned, even though only 1 key may have changed. This diff adds a new `UpdatesOnly` option, that when set, introduces server-side filtering to remove all keys/entries (from a `Keys()` or `List()` query) that have an old `ModifyIndex`.

It uses `MinQueryIndex` as the threshold here, which follows the effective serialization/deserialization chain: `WaitIndex` --> `index` --> `MinQueryIndex`.
- `WaitIndex` being the client-side parameter, determining the index the "client" waits for on a blocking query
- `index` being the serialized HTTP query parameter representing the `WaitIndex`
- `MinQueryIndex` being the server-side struct (deserialized result of `index`)

While we (potentially?) wait for the streaming backend: https://developer.hashicorp.com/consul/api-docs/features/blocking#streaming-backend to be implemented for KV endpoints, this can be a good stop-gap solution to enable in-memory, incremental scans or queries over a prefix.
@tanmayghai18 tanmayghai18 force-pushed the tanmayghai18/add-option-to-only-watch-changed-keys branch from 87a7836 to c9997a8 Compare January 28, 2025 22:25
reply.QueryMeta.ResultsFilteredByACLs = total != len(ent)
total := len(entries)
entries = FilterDirEnt(authz, entries)
reply.QueryMeta.ResultsFilteredByACLs = total != len(entries)
Copy link
Author

@tanmayghai18 tanmayghai18 Jan 28, 2025

Choose a reason for hiding this comment

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

open q: similar to how we hydrate ResultsFilteredByACLs in the QueryMeta introduced by #11569, does it make sense to do the same thing here for filtering by query index?

@tanmayghai18 tanmayghai18 marked this pull request as ready for review January 28, 2025 22:29
@tanmayghai18 tanmayghai18 requested a review from a team as a code owner January 28, 2025 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/api Relating to the HTTP API interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant