From 64c29d8051a3a435a5b98abb55e4c03dde30363c Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Fri, 23 Jun 2023 12:43:14 -0400 Subject: [PATCH 01/11] mage: add bench target --- magefiles/test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/magefiles/test.go b/magefiles/test.go index b6d0f7a4ee..ddd185ae45 100644 --- a/magefiles/test.go +++ b/magefiles/test.go @@ -53,6 +53,11 @@ func (Test) Analyzers() error { return goDirTest("./tools/analyzers", "./...") } +// Bench Runs benchmarks +func (Test) Bench() error { + return goTest("-bench=.", "-benchmem", "-run=^$", "-tags=ci,docker,skipintegrationtests", "-timeout=10m", "./...") +} + // Wasm Run wasm browser tests func (Test) Wasm() error { // build the test binary From 3188aae17a6fe409fad207e99995c59461d34d4f Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Fri, 23 Jun 2023 13:12:33 -0400 Subject: [PATCH 02/11] .github: add workflow for benchmarks --- .github/workflows/benchmark.yaml | 41 ++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 .github/workflows/benchmark.yaml diff --git a/.github/workflows/benchmark.yaml b/.github/workflows/benchmark.yaml new file mode 100644 index 0000000000..11853c2cd3 --- /dev/null +++ b/.github/workflows/benchmark.yaml @@ -0,0 +1,41 @@ +--- +name: "Benchmarks" +on: # yamllint disable-line rule:truthy + push: + branches: + - "main" + pull_request: + branches: + - "*" + merge_group: + types: + - "checks_requested" +env: + GO_VERSION: "~1.20.5" +jobs: + go-bench: + name: "Go Benchmark" + runs-on: ["self-hosted", "benchmarks"] + needs: "unit" + steps: + - uses: "actions/checkout@v3" + - uses: "authzed/actions/setup-go@main" + with: + go-version: "${{ env.GO_VERSION }}" + - name: "Benchmark" + run: "go run mage.go test:bench | tee benchmarks.txt" + - name: "Download previous benchmark data" + uses: "actions/cache@v1" + with: + path: "./cache" + key: "${{ runner.os }}-benchmark" + - uses: "benchmark-action/github-action-benchmark@v1" + with: + tool: "go" + output-file-path: "benchmarks.txt" + save-data-file: "${{ github.ref_name == 'main' }}" + external-data-json-path: "./cache/benchmark-data.json" + github-token: "${{ github.token }}" + auto-push: false + comment-on-alert: true + summary-always: true From ed2c02ab3236d06da2d17bc61d6d650c208b2989 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Fri, 23 Jun 2023 13:25:54 -0400 Subject: [PATCH 03/11] .github: only run go lint when go changes --- .DS_Store | Bin 0 -> 6148 bytes .github/workflows/lint.yaml | 13 ++ tools/analyzers/go.work.sum | 232 ++++++++++++++++++++++++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 .DS_Store diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..7eef725885f858d68f9c9e763256763de753fc30 GIT binary patch literal 6148 zcmeHK%}T>S5Z<+|O({YS3Oz1(Em}(zi-EVe&b~7Jje;8wYG!Kp#vl(L+G(?U{g`m08wPk`4xthcC^K|OR(yy52 zZ<_Gi8|;8ZEMOs9{r(Ss8b@i?>wWUATD`H|w3=4ix_6&s?q*&#Pedkg8l)JFJ$~n%JGiqCY(8ib1cx?25&( z-|LFg!SQn0vi9~5&n_p=@k=7#G?5$_SF&TUgm+L@DtdKiNi35`u$CERBq1?C3=jjv zz-BUF&I7BxndMW}!~iky0|U4}2xy3o!9t_jI-tYrGy3a@D4^q80#O)r3>F%}1HyGG zpibrHiNSR`*oBF63>F%7I^$|(n8&PKK3=$59qdAdGwx`lo){no))}bjp^fMN1^hCV zkNou%8W97;z&~Sv*T?R71Vx#%^;>y()(U73&`>b0Km`Q!wMzgDa3AR?r}hifA Date: Fri, 23 Jun 2023 13:33:05 -0400 Subject: [PATCH 04/11] .github: move benchmark into build-test --- .github/workflows/benchmark.yaml | 41 ------------------------------- .github/workflows/build-test.yaml | 28 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 41 deletions(-) delete mode 100644 .github/workflows/benchmark.yaml diff --git a/.github/workflows/benchmark.yaml b/.github/workflows/benchmark.yaml deleted file mode 100644 index 11853c2cd3..0000000000 --- a/.github/workflows/benchmark.yaml +++ /dev/null @@ -1,41 +0,0 @@ ---- -name: "Benchmarks" -on: # yamllint disable-line rule:truthy - push: - branches: - - "main" - pull_request: - branches: - - "*" - merge_group: - types: - - "checks_requested" -env: - GO_VERSION: "~1.20.5" -jobs: - go-bench: - name: "Go Benchmark" - runs-on: ["self-hosted", "benchmarks"] - needs: "unit" - steps: - - uses: "actions/checkout@v3" - - uses: "authzed/actions/setup-go@main" - with: - go-version: "${{ env.GO_VERSION }}" - - name: "Benchmark" - run: "go run mage.go test:bench | tee benchmarks.txt" - - name: "Download previous benchmark data" - uses: "actions/cache@v1" - with: - path: "./cache" - key: "${{ runner.os }}-benchmark" - - uses: "benchmark-action/github-action-benchmark@v1" - with: - tool: "go" - output-file-path: "benchmarks.txt" - save-data-file: "${{ github.ref_name == 'main' }}" - external-data-json-path: "./cache/benchmark-data.json" - github-token: "${{ github.token }}" - auto-push: false - comment-on-alert: true - summary-always: true diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 97b48ae783..75d6930bf0 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -220,3 +220,31 @@ jobs: input: "proto/internal" against: "https://github.com/authzed/spicedb.git#branch=main,subdir=proto/internal" buf_token: "${{ secrets.BUF_REGISTRY_TOKEN }}" + go-bench: + name: "Go Benchmark" + runs-on: ["self-hosted", "benchmarks"] + needs: "paths-filter" + if: | + needs.paths-filter.outputs.codechange == 'true' + steps: + - uses: "actions/checkout@v3" + - uses: "authzed/actions/setup-go@main" + with: + go-version: "${{ env.GO_VERSION }}" + - name: "Benchmark" + run: "go run mage.go test:bench | tee benchmarks.txt" + - name: "Download previous benchmark data" + uses: "actions/cache@v1" + with: + path: "./cache" + key: "${{ runner.name }}-benchmark-main" + - uses: "benchmark-action/github-action-benchmark@v1" + with: + tool: "go" + output-file-path: "benchmarks.txt" + save-data-file: "${{ github.ref_name == 'main' }}" + external-data-json-path: "./cache/benchmark-data.json" + github-token: "${{ github.token }}" + auto-push: false + comment-on-alert: true + summary-always: true From 1026348d4044b1976449f0c985b5752dd062e70f Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Fri, 23 Jun 2023 14:25:10 -0400 Subject: [PATCH 05/11] .github: set pipefail for benchmarks This avoids tee overriding the exit code when a benchmark fails. --- .github/workflows/build-test.yaml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index 75d6930bf0..d5442162f0 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -232,7 +232,9 @@ jobs: with: go-version: "${{ env.GO_VERSION }}" - name: "Benchmark" - run: "go run mage.go test:bench | tee benchmarks.txt" + run: | + set -o pipefail # Don't let tee replace the exit-code + go run mage.go test:bench | tee benchmarks.txt - name: "Download previous benchmark data" uses: "actions/cache@v1" with: From dbab78592ec12cd0a09df830760dff4e99c9e2e0 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Mon, 3 Jul 2023 12:21:51 -0400 Subject: [PATCH 06/11] internal: add missing exposed port for mysql --- internal/testserver/datastore/mysql.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/testserver/datastore/mysql.go b/internal/testserver/datastore/mysql.go index 0710cccf73..aa648867b3 100644 --- a/internal/testserver/datastore/mysql.go +++ b/internal/testserver/datastore/mysql.go @@ -22,6 +22,7 @@ import ( const ( mysqlPort = 3306 + mysqlPortStr = "3306/tcp" defaultCreds = "root:secret" testDBPrefix = "spicedb_test_" ) @@ -61,6 +62,7 @@ func RunMySQLForTestingWithOptions(t testing.TB, options MySQLTesterOptions, bri Repository: "mysql", Tag: containerImageTag, Env: []string{"MYSQL_ROOT_PASSWORD=secret"}, + ExposedPorts: []string{mysqlPortStr}, // increase max connections (default 151) to accommodate tests using the same docker container Cmd: []string{"--max-connections=500"}, NetworkID: bridgeNetworkName, @@ -75,7 +77,7 @@ func RunMySQLForTestingWithOptions(t testing.TB, options MySQLTesterOptions, bri require.NoError(t, pool.Purge(resource)) }) - port := resource.GetPort(fmt.Sprintf("%d/tcp", mysqlPort)) + port := resource.GetPort(mysqlPortStr) if bridgeNetworkName != "" { builder.hostname = name builder.port = fmt.Sprintf("%d", mysqlPort) From abecf12808995fba1a6c50b2d051445e4f2245c2 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Mon, 3 Jul 2023 18:26:56 -0400 Subject: [PATCH 07/11] testserver/datastore: misc docker arg cleanup --- internal/testserver/datastore/crdb.go | 12 ++++-------- internal/testserver/datastore/mysql.go | 25 ++++++++++++------------- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/internal/testserver/datastore/crdb.go b/internal/testserver/datastore/crdb.go index be61279a18..1ed29fa714 100644 --- a/internal/testserver/datastore/crdb.go +++ b/internal/testserver/datastore/crdb.go @@ -46,24 +46,20 @@ func RunCRDBForTesting(t testing.TB, bridgeNetworkName string) RunningEngineForT NetworkID: bridgeNetworkName, }) require.NoError(t, err) - - builder := &crdbTester{ - hostname: "localhost", - creds: "root:fake", - } t.Cleanup(func() { require.NoError(t, pool.Purge(resource)) }) - port := resource.GetPort(fmt.Sprintf("%d/tcp", 26257)) + builder := &crdbTester{creds: "root:fake"} if bridgeNetworkName != "" { builder.hostname = name builder.port = "26257" } else { - builder.port = port + builder.hostname = "localhost" + builder.port = resource.GetPort("26257/tcp") } - uri := fmt.Sprintf("postgres://%s@localhost:%s/defaultdb?sslmode=disable", builder.creds, port) + uri := fmt.Sprintf("postgres://%s@localhost:%s/defaultdb?sslmode=disable", builder.creds, builder.port) require.NoError(t, pool.Retry(func() error { var err error ctx, cancelConnect := context.WithTimeout(context.Background(), dockerBootTimeout) diff --git a/internal/testserver/datastore/mysql.go b/internal/testserver/datastore/mysql.go index aa648867b3..531c444134 100644 --- a/internal/testserver/datastore/mysql.go +++ b/internal/testserver/datastore/mysql.go @@ -21,8 +21,8 @@ import ( ) const ( - mysqlPort = 3306 - mysqlPortStr = "3306/tcp" + mysqlPort = "3306" + mysqlPortPair = mysqlPort + "/tcp" defaultCreds = "root:secret" testDBPrefix = "spicedb_test_" ) @@ -62,30 +62,29 @@ func RunMySQLForTestingWithOptions(t testing.TB, options MySQLTesterOptions, bri Repository: "mysql", Tag: containerImageTag, Env: []string{"MYSQL_ROOT_PASSWORD=secret"}, - ExposedPorts: []string{mysqlPortStr}, - // increase max connections (default 151) to accommodate tests using the same docker container - Cmd: []string{"--max-connections=500"}, - NetworkID: bridgeNetworkName, + ExposedPorts: []string{mysqlPortPair}, + Cmd: []string{"--max-connections=500"}, // accommodate tests using the same container + NetworkID: bridgeNetworkName, }) require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, pool.Purge(resource)) + }) builder := &mysqlTester{ creds: defaultCreds, options: options, } - t.Cleanup(func() { - require.NoError(t, pool.Purge(resource)) - }) - port := resource.GetPort(mysqlPortStr) if bridgeNetworkName != "" { builder.hostname = name - builder.port = fmt.Sprintf("%d", mysqlPort) + builder.port = mysqlPort } else { - builder.port = port + builder.hostname = "localhost" + builder.port = resource.GetPort(mysqlPortPair) } - dsn := fmt.Sprintf("%s@(localhost:%s)/mysql?parseTime=true", builder.creds, port) + dsn := fmt.Sprintf("%s@(localhost:%s)/mysql?parseTime=true", builder.creds, builder.port) require.NoError(t, pool.Retry(func() error { var err error builder.db, err = sql.Open("mysql", dsn) From a67aa2bb94eb0206df4836906490b8857b933119 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Fri, 7 Jul 2023 16:19:03 -0400 Subject: [PATCH 08/11] testserver/datastore: default to mysql8, check arm --- internal/testserver/datastore/mysql.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/testserver/datastore/mysql.go b/internal/testserver/datastore/mysql.go index 531c444134..f2cb26d040 100644 --- a/internal/testserver/datastore/mysql.go +++ b/internal/testserver/datastore/mysql.go @@ -7,6 +7,7 @@ import ( "context" "database/sql" "fmt" + "runtime" "testing" "github.com/google/uuid" @@ -61,6 +62,7 @@ func RunMySQLForTestingWithOptions(t testing.TB, options MySQLTesterOptions, bri Name: name, Repository: "mysql", Tag: containerImageTag, + Platform: "linux/amd64", // mysql:5 image does not have arm support Env: []string{"MYSQL_ROOT_PASSWORD=secret"}, ExposedPorts: []string{mysqlPortPair}, Cmd: []string{"--max-connections=500"}, // accommodate tests using the same container From d9cbea4b4dee56f37e9be1da320208a137491242 Mon Sep 17 00:00:00 2001 From: Jimmy Zelinskie Date: Fri, 14 Jul 2023 12:11:38 -0700 Subject: [PATCH 09/11] wip: fix error string --- pkg/datastore/test/tuples.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pkg/datastore/test/tuples.go b/pkg/datastore/test/tuples.go index 194207dce4..a246dfcdde 100644 --- a/pkg/datastore/test/tuples.go +++ b/pkg/datastore/test/tuples.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "strconv" + "strings" "sync" "testing" "time" @@ -600,7 +601,10 @@ func CreateAlreadyExistingTest(t *testing.T, tester DatastoreTester) { _, err = common.WriteTuples(ctx, ds, core.RelationTupleUpdate_CREATE, tpl1) require.ErrorAs(err, &common.CreateRelationshipExistsError{}) - require.Contains(err.Error(), "could not CREATE relationship ") + require.Condition(func() bool { + return strings.Contains(err.Error(), "could not CREATE relationship") || // mysql5 + strings.Contains(err.Error(), "could not CREATE one or more relationships") // mysql8 + }) grpcutil.RequireStatus(t, codes.AlreadyExists, err) f := func(ctx context.Context, rwt datastore.ReadWriteTransaction) error { From 24ac7a31e8a02a80670744a37d944dc9419f5769 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Wed, 20 Mar 2024 10:26:58 +0000 Subject: [PATCH 10/11] use latest Go version always --- .github/workflows/build-test.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/build-test.yaml b/.github/workflows/build-test.yaml index d5442162f0..230b782495 100644 --- a/.github/workflows/build-test.yaml +++ b/.github/workflows/build-test.yaml @@ -229,8 +229,6 @@ jobs: steps: - uses: "actions/checkout@v3" - uses: "authzed/actions/setup-go@main" - with: - go-version: "${{ env.GO_VERSION }}" - name: "Benchmark" run: | set -o pipefail # Don't let tee replace the exit-code From 9dbdafb422fc190e13625fdb0b20607db2f76f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C3=ADctor=20Rold=C3=A1n=20Betancort?= Date: Wed, 20 Mar 2024 10:28:06 +0000 Subject: [PATCH 11/11] fix merge conflict: SpiceDB does no longer support MySQL 5 --- internal/testserver/datastore/mysql.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/testserver/datastore/mysql.go b/internal/testserver/datastore/mysql.go index f2cb26d040..746d2e96aa 100644 --- a/internal/testserver/datastore/mysql.go +++ b/internal/testserver/datastore/mysql.go @@ -7,7 +7,6 @@ import ( "context" "database/sql" "fmt" - "runtime" "testing" "github.com/google/uuid" @@ -24,8 +23,8 @@ import ( const ( mysqlPort = "3306" mysqlPortPair = mysqlPort + "/tcp" - defaultCreds = "root:secret" - testDBPrefix = "spicedb_test_" + defaultCreds = "root:secret" + testDBPrefix = "spicedb_test_" ) type mysqlTester struct { @@ -59,11 +58,10 @@ func RunMySQLForTestingWithOptions(t testing.TB, options MySQLTesterOptions, bri name := fmt.Sprintf("mysql-%s", uuid.New().String()) resource, err := pool.RunWithOptions(&dockertest.RunOptions{ - Name: name, - Repository: "mysql", - Tag: containerImageTag, - Platform: "linux/amd64", // mysql:5 image does not have arm support - Env: []string{"MYSQL_ROOT_PASSWORD=secret"}, + Name: name, + Repository: "mysql", + Tag: containerImageTag, + Env: []string{"MYSQL_ROOT_PASSWORD=secret"}, ExposedPorts: []string{mysqlPortPair}, Cmd: []string{"--max-connections=500"}, // accommodate tests using the same container NetworkID: bridgeNetworkName,