Skip to content

Commit 126eccd

Browse files
josephschorrjosephschorr
committed
perf(mysql): force per-query-shape indexes for relationship queries
MySQL's optimizer would sometimes choose a suboptimal index for relationship reads, and the prior index-to-query-shape mapping was inaccurate -- several shapes pointed at indexes that don't serve them well, with TODOs noting as much. Force the index that actually matches each query shape, and use the sort order that index provides. relation_tuple is clustered on the synthetic `id` primary key, so the resource-keyed UNIQUE index (uq_relation_tuple_living) plays the role that the primary key plays in CRDB/Postgres for forward query shapes. The mapping is: - uq_relation_tuple_living (ByResource): CheckPermissionSelectDirectSubjects, CheckPermissionSelectIndirectSubjects, AllSubjectsForResources, FindResourceOfType - ix_relation_tuple_by_subject (BySubject): MatchingResourcesForSubject - ix_relation_tuple_by_subject_relation (BySubject): FindResourceAndSubjectWithRelations, FindSubjectOfTypeAndRelation, FindResourceRelationForSubjectRelation IndexingHintForQueryShape returns a forcedIndex (which emits MySQL `FORCE INDEX (...)`) for these shapes and no hint otherwise. QueryRelationships and ReverseQueryRelationships apply the hint based on the request's query shape. The uq_relation_tuple_namespace UNIQUE index is now documented as a uniqueness-only constraint; it is not used to serve any read shape. Adds a TestIndexingHintForQueryShape unit test for the shape -> index/sort mapping and a BenchmarkMySQLRelQueries benchmark covering the affected forward and reverse query shapes.
1 parent 8908ebe commit 126eccd

5 files changed

Lines changed: 304 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
1010
### Changed
1111
- Cache: switch to [otter](https://maypok86.github.io/otter/) as the primary cache implementation (https://github.com/authzed/spicedb/pull/3112)
1212
- Server handles: `GRPCDialContext` as a handle on the server used deprecated gRPC methods. We modernized it and renamed it to `NewClient` (https://github.com/authzed/spicedb/pull/3147)
13+
- MySQL: relationship reads now force the index that matches each query shape (and its sort order), rather than relying on the MySQL optimizer to pick one. This avoids cases where the optimizer chose a suboptimal index for `Check`, `LookupResources`, `LookupSubjects`, and `ReadRelationships`/reverse-relationship queries. (https://github.com/authzed/spicedb/pull/3173)
1314

1415
### Fixed
1516
- The watching schema cache (`--enable-experimental-watchable-schema-cache`) no longer enters permanent fallback on transient watch errors. A new supervisor restarts the watch cycle with bounded exponential backoff and only treats caller-driven cancellation or unsupported-watch as terminal (https://github.com/authzed/spicedb/pull/3134)

‎internal/datastore/benchmark/mysql_rels_bench_test.go‎

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
package benchmark
2+
3+
import (
4+
"context"
5+
"strconv"
6+
"testing"
7+
"time"
8+
9+
"github.com/stretchr/testify/require"
10+
11+
"github.com/authzed/spicedb/internal/datastore/mysql"
12+
"github.com/authzed/spicedb/internal/testfixtures"
13+
testdatastore "github.com/authzed/spicedb/internal/testserver/datastore"
14+
"github.com/authzed/spicedb/internal/testserver/datastore/config"
15+
dsconfig "github.com/authzed/spicedb/pkg/cmd/datastore"
16+
"github.com/authzed/spicedb/pkg/datastore"
17+
"github.com/authzed/spicedb/pkg/datastore/options"
18+
"github.com/authzed/spicedb/pkg/datastore/queryshape"
19+
"github.com/authzed/spicedb/pkg/tuple"
20+
)
21+
22+
const (
23+
mysqlBenchNumDocuments = 5000
24+
mysqlBenchUsersPerDoc = 10
25+
)
26+
27+
// BenchmarkMySQLRelQueries benchmarks the relationship-query shapes affected by the
28+
// MySQL index-hint / pagination work. Requires Docker (spins up mysql:8.4).
29+
//
30+
// Run with:
31+
//
32+
// go test ./internal/datastore/benchmark/ -bench BenchmarkMySQLRelQueries \
33+
// -benchmem -run '^$' -timeout 30m
34+
func BenchmarkMySQLRelQueries(b *testing.B) {
35+
b.StopTimer()
36+
ctx := b.Context()
37+
38+
engine := testdatastore.RunDatastoreEngine(b, mysql.Engine)
39+
ds := engine.NewDatastore(b, config.DatastoreConfigInitFunc(
40+
b,
41+
dsconfig.WithRevisionQuantization(5*time.Second),
42+
dsconfig.WithGCWindow(2*time.Hour),
43+
dsconfig.WithGCInterval(1*time.Hour),
44+
dsconfig.WithWatchBufferLength(1000),
45+
dsconfig.WithWriteAcquisitionTimeout(5*time.Second),
46+
))
47+
b.Cleanup(func() { _ = ds.Close() })
48+
49+
ds, _ = testfixtures.StandardDatastoreWithSchema(b, ds)
50+
51+
// Write a meaningful amount of data. Subject user "0" ends up a viewer of every
52+
// document, which exercises the reverse (MatchingResourcesForSubject) shape well.
53+
for docNum := range mysqlBenchNumDocuments {
54+
_, err := ds.ReadWriteTx(ctx, func(ctx context.Context, rwt datastore.ReadWriteTransaction) error {
55+
updates := make([]tuple.RelationshipUpdate, 0, mysqlBenchUsersPerDoc)
56+
for userNum := range mysqlBenchUsersPerDoc {
57+
updates = append(updates, tuple.Create(docViewer(strconv.Itoa(docNum), strconv.Itoa(userNum))))
58+
}
59+
return rwt.WriteRelationships(ctx, updates)
60+
})
61+
require.NoError(b, err)
62+
}
63+
64+
time.Sleep(1 * time.Second)
65+
66+
headRevResult, err := ds.HeadRevision(ctx)
67+
require.NoError(b, err)
68+
headRev := headRevResult.Revision
69+
70+
drain := func(b *testing.B, iter datastore.RelationshipIterator) int {
71+
count := 0
72+
for _, err := range iter {
73+
require.NoError(b, err)
74+
count++
75+
}
76+
return count
77+
}
78+
79+
b.StartTimer()
80+
81+
// Forward: full resource + subject (direct check).
82+
b.Run("ForwardDirectCheck", func(b *testing.B) {
83+
for range b.N {
84+
iter, err := ds.SnapshotReader(headRev).QueryRelationships(ctx, datastore.RelationshipsFilter{
85+
OptionalResourceType: testfixtures.DocumentNS.Name,
86+
OptionalResourceIds: []string{"100"},
87+
OptionalResourceRelation: "viewer",
88+
OptionalSubjectsSelectors: []datastore.SubjectsSelector{{
89+
OptionalSubjectType: testfixtures.UserNS.Name,
90+
OptionalSubjectIds: []string{"0"},
91+
}},
92+
}, options.WithQueryShape(queryshape.CheckPermissionSelectDirectSubjects))
93+
require.NoError(b, err)
94+
require.Equal(b, 1, drain(b, iter))
95+
}
96+
})
97+
98+
// Forward: all subjects for one resource.
99+
b.Run("ForwardAllSubjects", func(b *testing.B) {
100+
for range b.N {
101+
iter, err := ds.SnapshotReader(headRev).QueryRelationships(ctx, datastore.RelationshipsFilter{
102+
OptionalResourceType: testfixtures.DocumentNS.Name,
103+
OptionalResourceIds: []string{"100"},
104+
OptionalResourceRelation: "viewer",
105+
}, options.WithQueryShape(queryshape.AllSubjectsForResources))
106+
require.NoError(b, err)
107+
require.Equal(b, mysqlBenchUsersPerDoc, drain(b, iter))
108+
}
109+
})
110+
111+
// Reverse: all resources for a specific subject (Finding 1 main target).
112+
b.Run("ReverseMatchingResources", func(b *testing.B) {
113+
for range b.N {
114+
iter, err := ds.SnapshotReader(headRev).ReverseQueryRelationships(ctx, datastore.SubjectsFilter{
115+
SubjectType: testfixtures.UserNS.Name,
116+
OptionalSubjectIds: []string{"0"},
117+
},
118+
options.WithResRelation(&options.ResourceRelation{
119+
Namespace: testfixtures.DocumentNS.Name,
120+
Relation: "viewer",
121+
}),
122+
options.WithSortForReverse(options.BySubject),
123+
options.WithQueryShapeForReverse(queryshape.MatchingResourcesForSubject),
124+
)
125+
require.NoError(b, err)
126+
require.Equal(b, mysqlBenchNumDocuments, drain(b, iter))
127+
}
128+
})
129+
130+
// Enumerate by type + relation, no object_id (Finding 3 candidate workload).
131+
b.Run("EnumerateByTypeAndRelation", func(b *testing.B) {
132+
for range b.N {
133+
iter, err := ds.SnapshotReader(headRev).QueryRelationships(ctx, datastore.RelationshipsFilter{
134+
OptionalResourceType: testfixtures.DocumentNS.Name,
135+
OptionalResourceRelation: "viewer",
136+
}, options.WithQueryShape(queryshape.Varying))
137+
require.NoError(b, err)
138+
require.Equal(b, mysqlBenchNumDocuments*mysqlBenchUsersPerDoc, drain(b, iter))
139+
}
140+
})
141+
142+
// Cursor-paginated read (Finding 4 target).
143+
b.Run("PaginatedRead", func(b *testing.B) {
144+
const pageSize = uint64(500)
145+
for range b.N {
146+
var after options.Cursor
147+
total := 0
148+
for {
149+
limit := pageSize
150+
opts := []options.QueryOptionsOption{
151+
options.WithSort(options.ByResource),
152+
options.WithLimit(&limit),
153+
options.WithQueryShape(queryshape.Varying),
154+
}
155+
if after != nil {
156+
opts = append(opts, options.WithAfter(after))
157+
}
158+
iter, err := ds.SnapshotReader(headRev).QueryRelationships(ctx, datastore.RelationshipsFilter{
159+
OptionalResourceType: testfixtures.DocumentNS.Name,
160+
OptionalResourceRelation: "viewer",
161+
}, opts...)
162+
require.NoError(b, err)
163+
164+
pageCount := 0
165+
var last tuple.Relationship
166+
for rel, err := range iter {
167+
require.NoError(b, err)
168+
last = rel
169+
pageCount++
170+
}
171+
total += pageCount
172+
if uint64(pageCount) < pageSize {
173+
break
174+
}
175+
lastCopy := last
176+
after = &lastCopy
177+
}
178+
require.Equal(b, mysqlBenchNumDocuments*mysqlBenchUsersPerDoc, total)
179+
}
180+
})
181+
}

‎internal/datastore/mysql/indexes.go‎

Lines changed: 71 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,54 +2,54 @@ package mysql
22

33
import (
44
"github.com/authzed/spicedb/internal/datastore/common"
5+
"github.com/authzed/spicedb/pkg/datastore/options"
56
"github.com/authzed/spicedb/pkg/datastore/queryshape"
67
)
78

8-
// IndexUniqueLivingRelationships is the UNIQUE constraint index on
9-
// living relationships.
9+
// IndexUniqueLivingRelationships is the UNIQUE constraint index on living relationships.
10+
// Because relation_tuple is clustered on the synthetic `id` primary key, this resource-keyed
11+
// index plays the role that the primary key plays in CRDB/Postgres for forward query shapes.
1012
var IndexUniqueLivingRelationships = common.IndexDefinition{
1113
Name: `uq_relation_tuple_living`,
1214
ColumnsSQL: `UNIQUE (namespace, object_id, relation, userset_namespace, userset_object_id, userset_relation, deleted_transaction)`,
1315
Shapes: []queryshape.Shape{
1416
queryshape.CheckPermissionSelectDirectSubjects,
17+
queryshape.CheckPermissionSelectIndirectSubjects,
18+
queryshape.AllSubjectsForResources,
19+
queryshape.FindResourceOfType,
1520
},
21+
PreferredSortOrder: options.ByResource,
1622
}
1723

18-
// IndexUniqueRelationships is the UNIQUE constraint index on all relationships.
24+
// IndexUniqueRelationships is the UNIQUE constraint index on all relationships (including
25+
// historical ones). It enforces uniqueness; it is not used as a read index for any query shape.
1926
var IndexUniqueRelationships = common.IndexDefinition{
2027
Name: `uq_relation_tuple_namespace`,
2128
ColumnsSQL: `UNIQUE (namespace, object_id, relation, userset_namespace, userset_object_id, userset_relation, created_transaction, deleted_transaction)`,
22-
Shapes: []queryshape.Shape{
23-
queryshape.CheckPermissionSelectDirectSubjects,
24-
queryshape.CheckPermissionSelectIndirectSubjects,
25-
},
2629
}
2730

28-
// IndexRelationshipBySubject is the index on the relationship table for
29-
// looking up relationships by subject.
31+
// IndexRelationshipBySubject is the index on the relationship table for looking up
32+
// relationships by subject (the reverse lookup).
3033
var IndexRelationshipBySubject = common.IndexDefinition{
3134
Name: `ix_relation_tuple_by_subject`,
3235
ColumnsSQL: `INDEX ix_relation_tuple_by_subject (userset_object_id, userset_namespace, userset_relation, namespace, relation)`,
3336
Shapes: []queryshape.Shape{
34-
// TODO: this index isn't great to use for this query shape, so we probably need to fix
35-
// the indexes
36-
queryshape.CheckPermissionSelectDirectSubjects,
37-
38-
queryshape.CheckPermissionSelectIndirectSubjects,
37+
queryshape.MatchingResourcesForSubject,
3938
},
39+
PreferredSortOrder: options.BySubject,
4040
}
4141

42-
// IndexRelationshipBySubjectRelation is the index on the relationship table for
43-
// looking up relationships by subject and relation.
42+
// IndexRelationshipBySubjectRelation is the index on the relationship table for looking up
43+
// relationships by subject type and relation.
4444
var IndexRelationshipBySubjectRelation = common.IndexDefinition{
4545
Name: `ix_relation_tuple_by_subject_relation`,
4646
ColumnsSQL: `INDEX ix_relation_tuple_by_subject_relation (userset_namespace, userset_relation, namespace, relation)`,
4747
Shapes: []queryshape.Shape{
48-
// TODO: this index isn't great to use for these query shape, so we probably need to fix
49-
// the indexes
50-
queryshape.CheckPermissionSelectDirectSubjects,
51-
queryshape.CheckPermissionSelectIndirectSubjects,
48+
queryshape.FindResourceAndSubjectWithRelations,
49+
queryshape.FindSubjectOfTypeAndRelation,
50+
queryshape.FindResourceRelationForSubjectRelation,
5251
},
52+
PreferredSortOrder: options.BySubject,
5353
}
5454

5555
// IndexRelationshipByDeletedTransaction is the index on the relationship table for
@@ -59,16 +59,14 @@ var IndexRelationshipByDeletedTransaction = common.IndexDefinition{
5959
ColumnsSQL: `INDEX ix_relation_tuple_by_deleted_transaction (deleted_transaction)`,
6060
}
6161

62-
// IndexRelationTupleWatch is the index on the relationship table for
63-
// watching relationships.
62+
// IndexRelationTupleWatch is the index on the relationship table for watching relationships.
6463
var IndexRelationTupleWatch = common.IndexDefinition{
6564
Name: `ix_relation_tuple_watch`,
6665
ColumnsSQL: `INDEX ix_relation_tuple_watch (created_transaction, deleted_transaction DESC)`,
6766
}
6867

69-
// IndexRelationTupleExpired is the index on the relationship table for
70-
// expired relationships. This is used for garbage collection of expired
71-
// relationships.
68+
// IndexRelationTupleExpired is the index on the relationship table for expired relationships.
69+
// This is used for garbage collection of expired relationships.
7270
var IndexRelationTupleExpired = common.IndexDefinition{
7371
Name: `ix_relation_tuple_expired`,
7472
ColumnsSQL: `INDEX ix_relation_tuple_expired (expiration)`,
@@ -83,3 +81,51 @@ var indexes = []common.IndexDefinition{
8381
IndexRelationTupleWatch,
8482
IndexRelationTupleExpired,
8583
}
84+
85+
// NoIndexingHint is the absence of an indexing hint.
86+
var NoIndexingHint common.IndexingHint = nil
87+
88+
// IndexingHintForQueryShape returns the index hint to force for a given query shape, if any.
89+
func IndexingHintForQueryShape(qs queryshape.Shape) common.IndexingHint {
90+
switch qs {
91+
case queryshape.CheckPermissionSelectDirectSubjects,
92+
queryshape.CheckPermissionSelectIndirectSubjects,
93+
queryshape.AllSubjectsForResources,
94+
queryshape.FindResourceOfType:
95+
return forcedIndex{IndexUniqueLivingRelationships}
96+
97+
case queryshape.MatchingResourcesForSubject:
98+
return forcedIndex{IndexRelationshipBySubject}
99+
100+
case queryshape.FindResourceAndSubjectWithRelations,
101+
queryshape.FindSubjectOfTypeAndRelation,
102+
queryshape.FindResourceRelationForSubjectRelation:
103+
return forcedIndex{IndexRelationshipBySubjectRelation}
104+
105+
default:
106+
return NoIndexingHint
107+
}
108+
}
109+
110+
// forcedIndex is an index hint that forces use of a specific index via MySQL's FORCE INDEX clause.
111+
type forcedIndex struct {
112+
index common.IndexDefinition
113+
}
114+
115+
func (f forcedIndex) FromSQLSuffix() (string, error) {
116+
return "FORCE INDEX (" + f.index.Name + ")", nil
117+
}
118+
119+
func (f forcedIndex) FromTable(existingTableName string) (string, error) {
120+
return existingTableName, nil
121+
}
122+
123+
func (f forcedIndex) SQLPrefix() (string, error) {
124+
return "", nil
125+
}
126+
127+
func (f forcedIndex) SortOrder() options.SortOrder {
128+
return f.index.PreferredSortOrder
129+
}
130+
131+
var _ common.IndexingHint = forcedIndex{}

‎internal/datastore/mysql/indexes_test.go‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package mysql
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"github.com/authzed/spicedb/pkg/datastore/options"
9+
"github.com/authzed/spicedb/pkg/datastore/queryshape"
10+
)
11+
12+
func TestIndexingHintForQueryShape(t *testing.T) {
13+
cases := map[queryshape.Shape]struct {
14+
indexName string
15+
sort options.SortOrder
16+
}{
17+
queryshape.CheckPermissionSelectDirectSubjects: {"uq_relation_tuple_living", options.ByResource},
18+
queryshape.CheckPermissionSelectIndirectSubjects: {"uq_relation_tuple_living", options.ByResource},
19+
queryshape.AllSubjectsForResources: {"uq_relation_tuple_living", options.ByResource},
20+
queryshape.FindResourceOfType: {"uq_relation_tuple_living", options.ByResource},
21+
queryshape.MatchingResourcesForSubject: {"ix_relation_tuple_by_subject", options.BySubject},
22+
queryshape.FindResourceAndSubjectWithRelations: {"ix_relation_tuple_by_subject_relation", options.BySubject},
23+
queryshape.FindSubjectOfTypeAndRelation: {"ix_relation_tuple_by_subject_relation", options.BySubject},
24+
queryshape.FindResourceRelationForSubjectRelation: {"ix_relation_tuple_by_subject_relation", options.BySubject},
25+
}
26+
27+
for shape, expected := range cases {
28+
t.Run(string(shape), func(t *testing.T) {
29+
hint := IndexingHintForQueryShape(shape)
30+
require.NotNil(t, hint)
31+
32+
suffix, err := hint.FromSQLSuffix()
33+
require.NoError(t, err)
34+
require.Equal(t, "FORCE INDEX ("+expected.indexName+")", suffix)
35+
36+
prefix, err := hint.SQLPrefix()
37+
require.NoError(t, err)
38+
require.Empty(t, prefix)
39+
40+
require.Equal(t, expected.sort, hint.SortOrder())
41+
})
42+
}
43+
44+
require.Nil(t, IndexingHintForQueryShape(queryshape.Varying))
45+
require.Nil(t, IndexingHintForQueryShape(queryshape.Unspecified))
46+
}

0 commit comments

Comments
 (0)