From 15619f564e7b8bc48d9cdacdcae02bf516f876ce Mon Sep 17 00:00:00 2001 From: Eugene Yakhnenko Date: Fri, 10 Jul 2026 10:49:44 -0700 Subject: [PATCH 1/3] docs(adr): propose ADR for bounded attribute value responses Adds a proposed ADR evaluating five options for addressing unbounded JSON_AGG on attribute values in the Attributes Service (issue #3744). Co-Authored-By: Claude Opus 4.6 Signed-off-by: Eugene Yakhnenko --- ...0-attributes-v2-bounded-value-responses.md | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md diff --git a/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md b/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md new file mode 100644 index 0000000000..eb03d70e7d --- /dev/null +++ b/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md @@ -0,0 +1,110 @@ +--- +status: proposed +date: 2026-07-10 +tags: + - policy + - attributes + - pagination + - performance +driver: Eugene Yakhnenko +--- +# Bounded attribute value responses in the Attributes Service + +## Context and Problem Statement + +The `AttributesService` RPCs `ListAttributes`, `GetAttribute`, and `ListAttributesByFqns` return **all** attribute values inline via unbounded `JSON_AGG` in SQL. There is no pagination or limit on the number of values returned per attribute. + +An attribute with a large number of values (thousands or more) causes: +- High memory pressure on Postgres during aggregation +- Oversized gRPC responses that stress client and server memory +- High latency on what should be lightweight list/get operations + +The dedicated `ListAttributeValues` RPC was removed in [PR #3108](https://github.com/opentdf/platform/pull/3108) as "redundant," leaving no paginated way to retrieve values independently. + +See [issue #3744](https://github.com/opentdf/platform/issues/3744) for full details. + +## Decision Drivers + +* Must eliminate unbounded `JSON_AGG` on attribute values in Postgres queries +* Must provide a paginated way to retrieve attribute values +* Must not break existing v1 API consumers +* Should follow existing platform patterns (e.g., `ListRegisteredResourceValues` for paginated sub-resources, authorization v2 for service versioning) +* Should allow fetching values across multiple attributes in a single call to avoid N+1 round trips + +## Considered Options + +1. **Modify v1 RPCs to add pagination on values**: add a `values_pagination` field to `ListAttributesRequest` and `GetAttributeRequest` to limit and paginate inline values +2. **v2 service: keep values on GetAttribute with sub-pagination**: introduce a v2 with paginated inline values on `GetAttribute`, strip values from `ListAttributes` +3. **v2 service: strip values entirely, add dedicated ListAttributeValues RPC**: v2 `ListAttributes` and `GetAttribute` return attributes without values; a new `ListAttributeValues` RPC provides paginated value access +4. **Application-layer cap**: add a `LIMIT` inside the `JSON_AGG` subquery (e.g., cap at 1000 values) without changing the API surface + +## Pros and Cons of the Options + +### Option 1: Modify v1 RPCs to add pagination on values + +Add optional `values_pagination` (PageRequest) to v1 `ListAttributesRequest` and `GetAttributeRequest`. Default behavior returns all values for backward compatibility; when pagination is provided, values are bounded. + +* 🟩 **Good**, because no new service version or RPCs needed +* 🟩 **Good**, because existing consumers continue to work unchanged (default returns all) +* 🟥 **Bad**, because the default (no pagination) still returns unbounded values: the dangerous path remains the easy path +* 🟥 **Bad**, because paginating values nested inside a list of attributes creates complex response semantics (each attribute has its own values pagination cursor) +* 🟥 **Bad**, because it doesn't solve the Postgres-side `JSON_AGG` cost: the query still joins and aggregates all values unless the SQL is fundamentally restructured + +### Option 2: v2 service with paginated inline values on GetAttribute + +Introduce a v2 `AttributesService`. `ListAttributes` returns attributes without values. `GetAttribute` returns the attribute with paginated values inline (values_pagination on the request, values + values_pagination on the response). + +* 🟩 **Good**, because `GetAttribute` is a single-resource call, so sub-pagination is less complex than on a list +* 🟩 **Good**, because consumers get attribute + first page of values in one call +* 🟥 **Bad**, because it still couples value fetching to the attribute fetch: the SQL query must join and aggregate values even when the caller only needs attribute metadata +* 🟥 **Bad**, because `ListAttributes` with 1000 attributes each having even a small page of values (e.g., 10) still produces a large response (10,000 value records) +* 🟥 **Bad**, because paginating sub-resources inline has no existing pattern in the platform to follow + +### Option 3: v2 service with dedicated ListAttributeValues RPC + +Introduce a v2 `AttributesService` with three RPCs: +- `ListAttributes`: returns attributes **without** values +- `GetAttribute`: returns a single attribute **without** values +- `ListAttributeValues`: paginated values with filters (state, sort, search), accepts multiple attribute IDs + +Consumers call `ListAttributes` or `GetAttribute` for attribute metadata, then `ListAttributeValues` for values when needed. + +* 🟩 **Good**, because attribute queries are fully decoupled from value queries: no `JSON_AGG` on values in any attribute query +* 🟩 **Good**, because `ListAttributeValues` follows the existing `ListRegisteredResourceValues` platform pattern +* 🟩 **Good**, because accepting `repeated attribute_ids` allows fetching values across multiple attributes in one paginated call, avoiding N+1 round trips +* 🟩 **Good**, because v2 service versioning follows the existing authorization v2 pattern: non-breaking, v1 continues to work +* 🟨 **Neutral**, because consumers need two calls instead of one to get an attribute with its values +* 🟥 **Bad**, because it introduces new SQL queries that partially duplicate existing ones (attribute queries minus the values join) + +### Option 4: Application-layer cap on JSON_AGG + +Add `LIMIT N` inside the `JSON_AGG` subquery or use `array_agg` with a cap. No API changes. + +* 🟩 **Good**, because zero API surface change: fully transparent to consumers +* 🟩 **Good**, because minimal code change +* 🟥 **Bad**, because it silently truncates data: consumers don't know values were dropped and have no way to fetch the rest +* 🟥 **Bad**, because there is no pagination mechanism to retrieve beyond the cap +* 🟥 **Bad**, because it masks the problem rather than solving it: the query still joins all values, just discards some after aggregation + +### Option 5: Do nothing + +Accept the current behavior. Attribute values are returned inline and unbounded. Operators are expected to manage attribute cardinality through policy governance (i.e., don't create attributes with millions of values). + +* 🟩 **Good**, because zero engineering effort +* 🟩 **Good**, because no API changes, no migration, no new queries +* 🟥 **Bad**, because the platform has no guardrail: a single high-cardinality attribute can degrade the entire service +* 🟥 **Bad**, because there is no paginated way to retrieve values after the removal of `ListAttributeValues` in PR #3108 +* 🟥 **Bad**, because it shifts the burden to operators who may not be aware of the risk until an outage occurs + +## Validation + +{To be filled after decision is accepted: describe how compliance with the ADR is validated, e.g., integration tests, load tests against large value sets.} + +## More Information + +* Issue: [opentdf/platform#3744](https://github.com/opentdf/platform/issues/3744): Unbounded attribute value aggregation in ListAttributes/GetAttribute +* PR #3108 removed `ListAttributeValues` as "redundant" +* Existing platform patterns: + - `ListRegisteredResourceValues`: paginated sub-resource listing pattern + - `authorization/v2`: v2 service versioning pattern + - `PageRequest`/`PageResponse`: shared pagination types in `policy/selectors.proto` From a890af7bf1b1451b075445cfa6fd5445bafd8bb2 Mon Sep 17 00:00:00 2001 From: Eugene Yakhnenko Date: Fri, 10 Jul 2026 11:04:59 -0700 Subject: [PATCH 2/3] docs(adr): refine ADR copy and reject options 1 and 4 Reduce verbosity, add rejected alternatives section, clarify that options 1 and 4 are breaking changes. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Eugene Yakhnenko --- ...0-attributes-v2-bounded-value-responses.md | 118 ++++++++---------- 1 file changed, 54 insertions(+), 64 deletions(-) diff --git a/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md b/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md index eb03d70e7d..b1dcf67c53 100644 --- a/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md +++ b/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md @@ -12,99 +12,89 @@ driver: Eugene Yakhnenko ## Context and Problem Statement -The `AttributesService` RPCs `ListAttributes`, `GetAttribute`, and `ListAttributesByFqns` return **all** attribute values inline via unbounded `JSON_AGG` in SQL. There is no pagination or limit on the number of values returned per attribute. +`ListAttributes`, `GetAttribute`, and `ListAttributesByFqns` return all attribute values inline via unbounded `JSON_AGG`. High-cardinality attributes cause Postgres memory pressure, oversized gRPC responses, and high latency. -An attribute with a large number of values (thousands or more) causes: -- High memory pressure on Postgres during aggregation -- Oversized gRPC responses that stress client and server memory -- High latency on what should be lightweight list/get operations +`ListAttributeValues` was removed in [PR #3108](https://github.com/opentdf/platform/pull/3108), leaving no paginated way to fetch values. -The dedicated `ListAttributeValues` RPC was removed in [PR #3108](https://github.com/opentdf/platform/pull/3108) as "redundant," leaving no paginated way to retrieve values independently. - -See [issue #3744](https://github.com/opentdf/platform/issues/3744) for full details. +See [issue #3744](https://github.com/opentdf/platform/issues/3744). ## Decision Drivers -* Must eliminate unbounded `JSON_AGG` on attribute values in Postgres queries -* Must provide a paginated way to retrieve attribute values -* Must not break existing v1 API consumers -* Should follow existing platform patterns (e.g., `ListRegisteredResourceValues` for paginated sub-resources, authorization v2 for service versioning) -* Should allow fetching values across multiple attributes in a single call to avoid N+1 round trips +* Eliminate unbounded `JSON_AGG` on attribute values +* Provide paginated value retrieval +* No breaking changes to v1 consumers +* Follow existing platform patterns (`ListRegisteredResourceValues`, authorization v2) +* Support multi-attribute value fetching in a single call ## Considered Options -1. **Modify v1 RPCs to add pagination on values**: add a `values_pagination` field to `ListAttributesRequest` and `GetAttributeRequest` to limit and paginate inline values -2. **v2 service: keep values on GetAttribute with sub-pagination**: introduce a v2 with paginated inline values on `GetAttribute`, strip values from `ListAttributes` -3. **v2 service: strip values entirely, add dedicated ListAttributeValues RPC**: v2 `ListAttributes` and `GetAttribute` return attributes without values; a new `ListAttributeValues` RPC provides paginated value access -4. **Application-layer cap**: add a `LIMIT` inside the `JSON_AGG` subquery (e.g., cap at 1000 values) without changing the API surface +1. **Modify v1 RPCs**: add `values_pagination` to existing requests +2. **v2 with inline sub-pagination**: v2 `GetAttribute` returns paginated values inline, `ListAttributes` strips values +3. **v2 with dedicated ListAttributeValues**: v2 strips values from all attribute RPCs, new `ListAttributeValues` RPC with pagination +4. **Application-layer cap**: `LIMIT` inside `JSON_AGG`, no API changes +5. **Do nothing**: accept current behavior ## Pros and Cons of the Options -### Option 1: Modify v1 RPCs to add pagination on values - -Add optional `values_pagination` (PageRequest) to v1 `ListAttributesRequest` and `GetAttributeRequest`. Default behavior returns all values for backward compatibility; when pagination is provided, values are bounded. +### Option 1: Modify v1 RPCs -* 🟩 **Good**, because no new service version or RPCs needed -* 🟩 **Good**, because existing consumers continue to work unchanged (default returns all) -* 🟥 **Bad**, because the default (no pagination) still returns unbounded values: the dangerous path remains the easy path -* 🟥 **Bad**, because paginating values nested inside a list of attributes creates complex response semantics (each attribute has its own values pagination cursor) -* 🟥 **Bad**, because it doesn't solve the Postgres-side `JSON_AGG` cost: the query still joins and aggregates all values unless the SQL is fundamentally restructured +Add optional `values_pagination` to v1 requests. Default returns all values for backward compatibility. -### Option 2: v2 service with paginated inline values on GetAttribute +* 🟩 **Good**, no new service version or RPCs +* 🟩 **Good**, existing consumers unaffected +* 🟥 **Bad**, default path still unbounded +* 🟥 **Bad**, per-attribute pagination cursors inside a list response is complex +* 🟥 **Bad**, Postgres still joins and aggregates all values unless SQL is restructured +* 🟥 **Bad**, using the optional field changes the response shape, making it a breaking change disguised as an optional parameter -Introduce a v2 `AttributesService`. `ListAttributes` returns attributes without values. `GetAttribute` returns the attribute with paginated values inline (values_pagination on the request, values + values_pagination on the response). +### Option 2: v2 with inline sub-pagination on GetAttribute -* 🟩 **Good**, because `GetAttribute` is a single-resource call, so sub-pagination is less complex than on a list -* 🟩 **Good**, because consumers get attribute + first page of values in one call -* 🟥 **Bad**, because it still couples value fetching to the attribute fetch: the SQL query must join and aggregate values even when the caller only needs attribute metadata -* 🟥 **Bad**, because `ListAttributes` with 1000 attributes each having even a small page of values (e.g., 10) still produces a large response (10,000 value records) -* 🟥 **Bad**, because paginating sub-resources inline has no existing pattern in the platform to follow +v2 `ListAttributes` strips values. v2 `GetAttribute` returns attribute + paginated values. -### Option 3: v2 service with dedicated ListAttributeValues RPC +* 🟩 **Good**, single call returns attribute + first page of values +* 🟥 **Bad**, SQL still joins values even when caller only needs attribute metadata +* 🟥 **Bad**, no existing platform pattern for inline sub-resource pagination -Introduce a v2 `AttributesService` with three RPCs: -- `ListAttributes`: returns attributes **without** values -- `GetAttribute`: returns a single attribute **without** values -- `ListAttributeValues`: paginated values with filters (state, sort, search), accepts multiple attribute IDs +### Option 3: v2 with dedicated ListAttributeValues RPC -Consumers call `ListAttributes` or `GetAttribute` for attribute metadata, then `ListAttributeValues` for values when needed. +v2 `ListAttributes` and `GetAttribute` return attributes without values. Separate `ListAttributeValues` RPC with state/sort/search filters, accepts `repeated attribute_ids`. -* 🟩 **Good**, because attribute queries are fully decoupled from value queries: no `JSON_AGG` on values in any attribute query -* 🟩 **Good**, because `ListAttributeValues` follows the existing `ListRegisteredResourceValues` platform pattern -* 🟩 **Good**, because accepting `repeated attribute_ids` allows fetching values across multiple attributes in one paginated call, avoiding N+1 round trips -* 🟩 **Good**, because v2 service versioning follows the existing authorization v2 pattern: non-breaking, v1 continues to work -* 🟨 **Neutral**, because consumers need two calls instead of one to get an attribute with its values -* 🟥 **Bad**, because it introduces new SQL queries that partially duplicate existing ones (attribute queries minus the values join) +* 🟩 **Good**, attribute queries fully decoupled from values: no `JSON_AGG` +* 🟩 **Good**, follows `ListRegisteredResourceValues` pattern +* 🟩 **Good**, multi-attribute fetch in one paginated call avoids N+1 +* 🟩 **Good**, v2 versioning follows authorization v2 pattern, non-breaking +* 🟨 **Neutral**, consumers need two calls for attribute + values +* 🟥 **Bad**, new SQL queries partially duplicate existing ones (minus values join) -### Option 4: Application-layer cap on JSON_AGG +### Option 4: Application-layer cap -Add `LIMIT N` inside the `JSON_AGG` subquery or use `array_agg` with a cap. No API changes. +Cap values via `LIMIT` in `JSON_AGG` (truncate) or error when a threshold is exceeded. No API changes. -* 🟩 **Good**, because zero API surface change: fully transparent to consumers -* 🟩 **Good**, because minimal code change -* 🟥 **Bad**, because it silently truncates data: consumers don't know values were dropped and have no way to fetch the rest -* 🟥 **Bad**, because there is no pagination mechanism to retrieve beyond the cap -* 🟥 **Bad**, because it masks the problem rather than solving it: the query still joins all values, just discards some after aggregation +* 🟩 **Good**, minimal code change +* 🟥 **Bad**, breaking change either way: truncation drops data silently, erroring fails previously working calls +* 🟥 **Bad**, no pagination mechanism to retrieve beyond the cap +* 🟥 **Bad**, query still joins all values, cap is applied after aggregation ### Option 5: Do nothing -Accept the current behavior. Attribute values are returned inline and unbounded. Operators are expected to manage attribute cardinality through policy governance (i.e., don't create attributes with millions of values). +Operators manage attribute cardinality through governance. + +* 🟩 **Good**, zero effort +* 🟥 **Bad**, no guardrail: one high-cardinality attribute can degrade the service +* 🟥 **Bad**, no paginated value retrieval exists +* 🟥 **Bad**, risk is invisible to operators until an outage + +### Rejected Alternatives -* 🟩 **Good**, because zero engineering effort -* 🟩 **Good**, because no API changes, no migration, no new queries -* 🟥 **Bad**, because the platform has no guardrail: a single high-cardinality attribute can degrade the entire service -* 🟥 **Bad**, because there is no paginated way to retrieve values after the removal of `ListAttributeValues` in PR #3108 -* 🟥 **Bad**, because it shifts the burden to operators who may not be aware of the risk until an outage occurs +* **Option 1: Modify v1 RPCs**: the `values_pagination` field is optional, but using it changes the response shape. Breaking change disguised as an optional field. +* **Option 4: Application-layer cap**: breaking change either way: truncation drops data silently, erroring fails previously working calls. No pagination to retrieve beyond the cap. ## Validation -{To be filled after decision is accepted: describe how compliance with the ADR is validated, e.g., integration tests, load tests against large value sets.} +{To be filled after decision is accepted.} ## More Information -* Issue: [opentdf/platform#3744](https://github.com/opentdf/platform/issues/3744): Unbounded attribute value aggregation in ListAttributes/GetAttribute -* PR #3108 removed `ListAttributeValues` as "redundant" -* Existing platform patterns: - - `ListRegisteredResourceValues`: paginated sub-resource listing pattern - - `authorization/v2`: v2 service versioning pattern - - `PageRequest`/`PageResponse`: shared pagination types in `policy/selectors.proto` +* [Issue #3744](https://github.com/opentdf/platform/issues/3744) +* [PR #3108](https://github.com/opentdf/platform/pull/3108) removed `ListAttributeValues` +* Platform patterns: `ListRegisteredResourceValues`, `authorization/v2`, `PageRequest`/`PageResponse` From bbbe463e3a099e0fc07e8909797f06fa42686e11 Mon Sep 17 00:00:00 2001 From: Eugene Yakhnenko Date: Fri, 10 Jul 2026 11:13:46 -0700 Subject: [PATCH 3/3] docs(adr): address review feedback on attributes v2 ADR Add Decision Outcome section, cap repeated attribute_ids input, note ListAttributesByFqns replaced by ListAttributes with FQN filter, trim rejected alternatives, reclassify two-call cost as Bad, fix Option 4 SQL analysis. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Eugene Yakhnenko --- ...7-10-attributes-v2-bounded-value-responses.md | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md b/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md index b1dcf67c53..5ddb072eae 100644 --- a/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md +++ b/adr/decisions/2026-07-10-attributes-v2-bounded-value-responses.md @@ -63,7 +63,7 @@ v2 `ListAttributes` and `GetAttribute` return attributes without values. Separat * 🟩 **Good**, follows `ListRegisteredResourceValues` pattern * 🟩 **Good**, multi-attribute fetch in one paginated call avoids N+1 * 🟩 **Good**, v2 versioning follows authorization v2 pattern, non-breaking -* 🟨 **Neutral**, consumers need two calls for attribute + values +* 🟥 **Bad**, consumers need two calls for attribute + values * 🟥 **Bad**, new SQL queries partially duplicate existing ones (minus values join) ### Option 4: Application-layer cap @@ -73,7 +73,7 @@ Cap values via `LIMIT` in `JSON_AGG` (truncate) or error when a threshold is exc * 🟩 **Good**, minimal code change * 🟥 **Bad**, breaking change either way: truncation drops data silently, erroring fails previously working calls * 🟥 **Bad**, no pagination mechanism to retrieve beyond the cap -* 🟥 **Bad**, query still joins all values, cap is applied after aggregation +* 🟥 **Bad**, requires SQL restructuring (e.g., lateral subqueries) to avoid full aggregation before capping ### Option 5: Do nothing @@ -84,10 +84,18 @@ Operators manage attribute cardinality through governance. * 🟥 **Bad**, no paginated value retrieval exists * 🟥 **Bad**, risk is invisible to operators until an outage +## Decision Outcome + +Chosen option: **Option 3, v2 with dedicated ListAttributeValues RPC**. Attribute queries are fully decoupled from value queries, follows existing platform patterns, and is non-breaking. + +`repeated attribute_ids` is capped (e.g., 100) and validated server-side to prevent shifting unbounded pressure from output to input. Pagination applies across all matched values flattened. + +`ListAttributesByFqns` is replaced by v2 `ListAttributes` with an FQN filter. + ### Rejected Alternatives -* **Option 1: Modify v1 RPCs**: the `values_pagination` field is optional, but using it changes the response shape. Breaking change disguised as an optional field. -* **Option 4: Application-layer cap**: breaking change either way: truncation drops data silently, erroring fails previously working calls. No pagination to retrieve beyond the cap. +* **Option 1: Modify v1 RPCs**: breaking change disguised as an optional field. +* **Option 4: Application-layer cap**: breaking change with no pagination path forward. ## Validation