From 1b12b2a9f85c0496520371ddb45470e087247518 Mon Sep 17 00:00:00 2001 From: strantalis Date: Sat, 5 Sep 2026 07:22:24 -0400 Subject: [PATCH 1/2] perf(authz): isolate missing values with bounded batch retries Signed-off-by: strantalis --- service/internal/access/v2/entitleable.go | 65 +++++++++++-------- .../access/v2/entitleable_batch_test.go | 63 ++++++++++++++++++ 2 files changed, 101 insertions(+), 27 deletions(-) create mode 100644 service/internal/access/v2/entitleable_batch_test.go diff --git a/service/internal/access/v2/entitleable.go b/service/internal/access/v2/entitleable.go index e8dbc79ee7..a89d7acf73 100644 --- a/service/internal/access/v2/entitleable.go +++ b/service/internal/access/v2/entitleable.go @@ -149,37 +149,48 @@ func fetchEntitleableAttributes( return resp, false, nil } - // processBatch resolves a batch, falling back to per-FQN resolution on a batch NotFound. The - // server rejects the whole batch with NotFound if any requested FQN does not exist, so the retry - // keeps the values that DO exist and skips the missing ones (denied per-resource downstream). This - // preserves valid decisions in a multi-resource request that also references an unknown FQN. - processBatch := func(batch []string) error { - resp, err := getBatch(batch) - if err == nil { - return process(resp, batch) - } - if connect.CodeOf(err) != connect.CodeNotFound { - return fmt.Errorf("failed to get entitleable attributes by fqns: %w", err) - } - for _, fqn := range batch { - single, skip, ferr := fetchOne(fqn) - if ferr != nil { - return ferr + // Split sparse misses breadth-first so valid subsets are retained in batches. + // Bound failed batch probes before reverting to single-value lookups when many + // values are missing. This keeps dense misses close to the original call count. + const maxFailedBatchProbes = 8 + for start := 0; start < len(normalizedFQNs); start += maxEntitleableFQNsPerRequest { + end := min(start+maxEntitleableFQNsPerRequest, len(normalizedFQNs)) + pending := [][]string{normalizedFQNs[start:end]} + failedProbes := 0 + for len(pending) > 0 { + batch := pending[0] + pending = pending[1:] + if failedProbes >= maxFailedBatchProbes && len(batch) > 1 { + for _, fqn := range batch { + single, skip, err := fetchOne(fqn) + if err != nil { + return nil, nil, err + } + if skip { + continue + } + if err := process(single, []string{fqn}); err != nil { + return nil, nil, err + } + } + continue } - if skip { + resp, err := getBatch(batch) + if err == nil { + if err := process(resp, batch); err != nil { + return nil, nil, err + } continue } - if perr := process(single, []string{fqn}); perr != nil { - return perr + if connect.CodeOf(err) != connect.CodeNotFound { + return nil, nil, fmt.Errorf("failed to get entitleable attributes by fqns: %w", err) } - } - return nil - } - - for start := 0; start < len(normalizedFQNs); start += maxEntitleableFQNsPerRequest { - end := min(start+maxEntitleableFQNsPerRequest, len(normalizedFQNs)) - if err := processBatch(normalizedFQNs[start:end]); err != nil { - return nil, nil, err + if len(batch) == 1 { + continue + } + failedProbes++ + middle := len(batch) / 2 + pending = append(pending, batch[:middle], batch[middle:]) } } diff --git a/service/internal/access/v2/entitleable_batch_test.go b/service/internal/access/v2/entitleable_batch_test.go new file mode 100644 index 0000000000..cf340cc83b --- /dev/null +++ b/service/internal/access/v2/entitleable_batch_test.go @@ -0,0 +1,63 @@ +package access + +import ( + "fmt" + "testing" + + "connectrpc.com/connect" + "github.com/opentdf/platform/protocol/go/policy" + attrs "github.com/opentdf/platform/protocol/go/policy/attributes" + "github.com/stretchr/testify/require" +) + +func TestEntitleableBatchSplitsSparseMisses(t *testing.T) { + const definition = "https://scale.example/attr/department" + const total = 250 + for _, missing := range []int{-1, 0, total - 1, total} { + t.Run(fmt.Sprintf("missing-%d", missing), func(t *testing.T) { + fqns := make([]string, total) + for i := range fqns { + fqns[i] = fmt.Sprintf("%s/value/%d", definition, i) + } + fake := &fakeAttributesClient{respFunc: func(req *attrs.GetEntitleableAttributesByFqnsRequest) (*attrs.GetEntitleableAttributesByFqnsResponse, error) { + for _, fqn := range req.GetFqns() { + if missing == total || (missing >= 0 && fqn == fqns[missing]) { + return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("missing value")) + } + } + resp := &attrs.GetEntitleableAttributesByFqnsResponse{ + Definitions: map[string]*attrs.GetEntitleableAttributesByFqnsResponse_EntitleableDefinition{definition: {Rule: policy.AttributeRuleTypeEnum_ATTRIBUTE_RULE_TYPE_ENUM_ANY_OF}}, + FqnEntitleableAttributes: make(map[string]*attrs.GetEntitleableAttributesByFqnsResponse_EntitleableAttribute), + } + for _, fqn := range req.GetFqns() { + resp.FqnEntitleableAttributes[fqn] = &attrs.GetEntitleableAttributesByFqnsResponse_EntitleableAttribute{DefinitionFqn: definition, Value: &attrs.GetEntitleableAttributesByFqnsResponse_EntitleableValue{ValueId: fqn, Fqn: fqn}} + } + return resp, nil + }} + definitions, _, err := fetchEntitleableAttributes(t.Context(), newSDKWithAttributes(fake), fqns) + require.NoError(t, err) + switch missing { + case -1: + require.Len(t, fake.requests, 1) + require.Len(t, definitions[0].GetValues(), total) + case total: + require.Empty(t, definitions) + require.LessOrEqual(t, len(fake.requests), total+8) + default: + require.Len(t, definitions, 1) + require.Len(t, definitions[0].GetValues(), total-1) + require.LessOrEqual(t, len(fake.requests), 17) + } + }) + } +} + +func TestEntitleableBatchDoesNotRetryOtherFailures(t *testing.T) { + expected := connect.NewError(connect.CodeUnavailable, fmt.Errorf("policy unavailable")) + fake := &fakeAttributesClient{respFunc: func(*attrs.GetEntitleableAttributesByFqnsRequest) (*attrs.GetEntitleableAttributesByFqnsResponse, error) { + return nil, expected + }} + _, _, err := fetchEntitleableAttributes(t.Context(), newSDKWithAttributes(fake), []string{"https://scale.example/attr/a/value/one", "https://scale.example/attr/a/value/two"}) + require.ErrorIs(t, err, expected) + require.Len(t, fake.requests, 1) +} From 896e190fa6b2a721395d7962bdb6cf4b9214b75b Mon Sep 17 00:00:00 2001 From: strantalis Date: Sat, 5 Sep 2026 07:22:59 -0400 Subject: [PATCH 2/2] style(authz): satisfy batch retry lint checks Signed-off-by: strantalis --- service/internal/access/v2/entitleable.go | 3 ++- service/internal/access/v2/entitleable_batch_test.go | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/service/internal/access/v2/entitleable.go b/service/internal/access/v2/entitleable.go index a89d7acf73..d1e2e298c5 100644 --- a/service/internal/access/v2/entitleable.go +++ b/service/internal/access/v2/entitleable.go @@ -153,6 +153,7 @@ func fetchEntitleableAttributes( // Bound failed batch probes before reverting to single-value lookups when many // values are missing. This keeps dense misses close to the original call count. const maxFailedBatchProbes = 8 + const splitDivisor = 2 for start := 0; start < len(normalizedFQNs); start += maxEntitleableFQNsPerRequest { end := min(start+maxEntitleableFQNsPerRequest, len(normalizedFQNs)) pending := [][]string{normalizedFQNs[start:end]} @@ -189,7 +190,7 @@ func fetchEntitleableAttributes( continue } failedProbes++ - middle := len(batch) / 2 + middle := len(batch) / splitDivisor pending = append(pending, batch[:middle], batch[middle:]) } } diff --git a/service/internal/access/v2/entitleable_batch_test.go b/service/internal/access/v2/entitleable_batch_test.go index cf340cc83b..75c3b97f71 100644 --- a/service/internal/access/v2/entitleable_batch_test.go +++ b/service/internal/access/v2/entitleable_batch_test.go @@ -1,6 +1,7 @@ package access import ( + "errors" "fmt" "testing" @@ -22,7 +23,7 @@ func TestEntitleableBatchSplitsSparseMisses(t *testing.T) { fake := &fakeAttributesClient{respFunc: func(req *attrs.GetEntitleableAttributesByFqnsRequest) (*attrs.GetEntitleableAttributesByFqnsResponse, error) { for _, fqn := range req.GetFqns() { if missing == total || (missing >= 0 && fqn == fqns[missing]) { - return nil, connect.NewError(connect.CodeNotFound, fmt.Errorf("missing value")) + return nil, connect.NewError(connect.CodeNotFound, errors.New("missing value")) } } resp := &attrs.GetEntitleableAttributesByFqnsResponse{ @@ -53,7 +54,7 @@ func TestEntitleableBatchSplitsSparseMisses(t *testing.T) { } func TestEntitleableBatchDoesNotRetryOtherFailures(t *testing.T) { - expected := connect.NewError(connect.CodeUnavailable, fmt.Errorf("policy unavailable")) + expected := connect.NewError(connect.CodeUnavailable, errors.New("policy unavailable")) fake := &fakeAttributesClient{respFunc: func(*attrs.GetEntitleableAttributesByFqnsRequest) (*attrs.GetEntitleableAttributesByFqnsResponse, error) { return nil, expected }}