zitadel/internal/crypto/database/database_test.go
Silvan 9c3e5e467b
perf(query): remove transactions for queries (#8614)
# Which Problems Are Solved

Queries currently execute 3 statements, begin, query, commit

# How the Problems Are Solved

remove transaction handling from query methods in database package

# Additional Changes

- Bump versions of `core_grpc_dependencies`-receipt in Makefile

# Additional info

During load tests we saw a lot of idle transactions of `zitadel_queries`
application name which is the connection pool used to query data in
zitadel. Executed query:

`select query_start - xact_start, pid, application_name, backend_start,
xact_start, query_start, state_change, wait_event_type,
wait_event,substring(query, 1, 200) query from pg_stat_activity where
datname = 'zitadel' and state <> 'idle';`

Mostly the last query executed was `begin isolation level read committed
read only`.

example: 

```
    ?column?     |  pid  |      application_name      |         backend_start         |          xact_start           |          query_start          |         state_change          | wait_event_type |  wait_event  |                                                                                                  query                                                                                                   
-----------------+-------+----------------------------+-------------------------------+-------------------------------+-------------------------------+-------------------------------+-----------------+--------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 00:00:00        | 33030 | zitadel_queries            | 2024-10-16 16:25:53.906036+00 | 2024-10-16 16:30:19.191661+00 | 2024-10-16 16:30:19.191661+00 | 2024-10-16 16:30:19.19169+00  | Client          | ClientRead   | begin isolation level read committed read only
 00:00:00        | 33035 | zitadel_queries            | 2024-10-16 16:25:53.909629+00 | 2024-10-16 16:30:19.19179+00  | 2024-10-16 16:30:19.19179+00  | 2024-10-16 16:30:19.191805+00 | Client          | ClientRead   | begin isolation level read committed read only
 00:00:00.00412  | 33028 | zitadel_queries            | 2024-10-16 16:25:53.904247+00 | 2024-10-16 16:30:19.187734+00 | 2024-10-16 16:30:19.191854+00 | 2024-10-16 16:30:19.191964+00 | Client          | ClientRead   | SELECT created_at, event_type, "sequence", "position", payload, creator, "owner", instance_id, aggregate_type, aggregate_id, revision FROM eventstore.events2 WHERE instance_id = $1 AND aggregate_type 
 00:00:00.084662 | 33134 | zitadel_es_pusher          | 2024-10-16 16:29:54.979692+00 | 2024-10-16 16:30:19.178578+00 | 2024-10-16 16:30:19.26324+00  | 2024-10-16 16:30:19.263267+00 | Client          | ClientRead   | RELEASE SAVEPOINT cockroach_restart
 00:00:00.084768 | 33139 | zitadel_es_pusher          | 2024-10-16 16:29:54.979585+00 | 2024-10-16 16:30:19.180762+00 | 2024-10-16 16:30:19.26553+00  | 2024-10-16 16:30:19.265531+00 | LWLock          | WALWriteLock | commit
 00:00:00.077377 | 33136 | zitadel_es_pusher          | 2024-10-16 16:29:54.978582+00 | 2024-10-16 16:30:19.187883+00 | 2024-10-16 16:30:19.26526+00  | 2024-10-16 16:30:19.265431+00 | Client          | ClientRead   | WITH existing AS (                                                                                                                                                                                      +
                 |       |                            |                               |                               |                               |                               |                 |              |     (SELECT instance_id, aggregate_type, aggregate_id, "sequence" FROM eventstore.events2 WHERE instance_id = $1 AND aggregate_type = $2 AND aggregate_id = $3 ORDER BY "sequence" DE
 00:00:00.012309 | 33123 | zitadel_es_pusher          | 2024-10-16 16:29:54.963484+00 | 2024-10-16 16:30:19.175066+00 | 2024-10-16 16:30:19.187375+00 | 2024-10-16 16:30:19.187376+00 | IO              | WalSync      | commit
 00:00:00        | 33034 | zitadel_queries            | 2024-10-16 16:25:53.90791+00  | 2024-10-16 16:30:19.262921+00 | 2024-10-16 16:30:19.262921+00 | 2024-10-16 16:30:19.263133+00 | Client          | ClientRead   | begin isolation level read committed read only
 00:00:00        | 33039 | zitadel_queries            | 2024-10-16 16:25:53.914106+00 | 2024-10-16 16:30:19.191676+00 | 2024-10-16 16:30:19.191676+00 | 2024-10-16 16:30:19.191687+00 | Client          | ClientRead   | begin isolation level read committed read only
 00:00:00.24539  | 33083 | zitadel_projection_spooler | 2024-10-16 16:27:49.895548+00 | 2024-10-16 16:30:19.020058+00 | 2024-10-16 16:30:19.265448+00 | 2024-10-16 16:30:19.26546+00  | Client          | ClientRead   | SAVEPOINT exec_stmt
 00:00:00        | 33125 | zitadel_es_pusher          | 2024-10-16 16:29:54.963859+00 | 2024-10-16 16:30:19.191715+00 | 2024-10-16 16:30:19.191715+00 | 2024-10-16 16:30:19.191729+00 | Client          | ClientRead   | begin
 00:00:00.004292 | 33032 | zitadel_queries            | 2024-10-16 16:25:53.906624+00 | 2024-10-16 16:30:19.187713+00 | 2024-10-16 16:30:19.192005+00 | 2024-10-16 16:30:19.192062+00 | Client          | ClientRead   | SELECT created_at, event_type, "sequence", "position", payload, creator, "owner", instance_id, aggregate_type, aggregate_id, revision FROM eventstore.events2 WHERE instance_id = $1 AND aggregate_type 
 00:00:00        | 33031 | zitadel_queries            | 2024-10-16 16:25:53.906422+00 | 2024-10-16 16:30:19.191625+00 | 2024-10-16 16:30:19.191625+00 | 2024-10-16 16:30:19.191645+00 | Client          | ClientRead   | begin isolation level read committed read only

```

The amount of idle transactions is significantly less if the query
transactions are removed:

example: 

```
    ?column?     |  pid  |      application_name      |         backend_start         |          xact_start           |          query_start          |         state_change          | wait_event_type | wait_event |                                                                                                  query                                                                                                   
-----------------+-------+----------------------------+-------------------------------+-------------------------------+-------------------------------+-------------------------------+-----------------+------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 00:00:00.000094 | 32741 | zitadel_queries            | 2024-10-16 16:23:49.73935+00  | 2024-10-16 16:24:59.785589+00 | 2024-10-16 16:24:59.785683+00 | 2024-10-16 16:24:59.785684+00 |                 |            | SELECT created_at, event_type, "sequence", "position", payload, creator, "owner", instance_id, aggregate_type, aggregate_id, revision FROM eventstore.events2 WHERE instance_id = $1 AND aggregate_type 
 00:00:00        | 32762 | zitadel_es_pusher          | 2024-10-16 16:24:02.275136+00 | 2024-10-16 16:24:59.784586+00 | 2024-10-16 16:24:59.784586+00 | 2024-10-16 16:24:59.784607+00 | Client          | ClientRead | begin
 00:00:00.000167 | 32742 | zitadel_queries            | 2024-10-16 16:23:49.740489+00 | 2024-10-16 16:24:59.784274+00 | 2024-10-16 16:24:59.784441+00 | 2024-10-16 16:24:59.784442+00 |                 |            | with usr as (                                                                                                                                                                                           +
                 |       |                            |                               |                               |                               |                               |                 |            |         select u.id, u.creation_date, u.change_date, u.sequence, u.state, u.resource_owner, u.username, n.login_name as preferred_login_name                                                            +
                 |       |                            |                               |                               |                               |                               |                 |            |         from projections.users13 u                                                                                                                                                                      +
                 |       |                            |                               |                               |                               |                               |                 |            |         left join projections.l
 00:00:00.256014 | 32759 | zitadel_projection_spooler | 2024-10-16 16:24:01.418429+00 | 2024-10-16 16:24:59.52959+00  | 2024-10-16 16:24:59.785604+00 | 2024-10-16 16:24:59.785649+00 | Client          | ClientRead | UPDATE projections.milestones SET reached_date = $1 WHERE (instance_id = $2) AND (type = $3) AND (reached_date IS NULL)
 00:00:00.014199 | 32773 | zitadel_es_pusher          | 2024-10-16 16:24:02.320404+00 | 2024-10-16 16:24:59.769509+00 | 2024-10-16 16:24:59.783708+00 | 2024-10-16 16:24:59.783709+00 | IO              | WalSync    | commit
 00:00:00        | 32765 | zitadel_es_pusher          | 2024-10-16 16:24:02.28173+00  | 2024-10-16 16:24:59.780413+00 | 2024-10-16 16:24:59.780413+00 | 2024-10-16 16:24:59.780426+00 | Client          | ClientRead | begin
 00:00:00.012729 | 32777 | zitadel_es_pusher          | 2024-10-16 16:24:02.339737+00 | 2024-10-16 16:24:59.767432+00 | 2024-10-16 16:24:59.780161+00 | 2024-10-16 16:24:59.780195+00 | Client          | ClientRead | RELEASE SAVEPOINT cockroach_restart
```

---------

Co-authored-by: Tim Möhlmann <tim+github@zitadel.com>
Co-authored-by: Livio Spring <livio.a@gmail.com>
Co-authored-by: Max Peintner <max@caos.ch>
Co-authored-by: Elio Bischof <elio@zitadel.com>
Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>
Co-authored-by: Miguel Cabrerizo <30386061+doncicuto@users.noreply.github.com>
Co-authored-by: Joakim Lodén <Loddan@users.noreply.github.com>
Co-authored-by: Yxnt <Yxnt@users.noreply.github.com>
Co-authored-by: Stefan Benz <stefan@caos.ch>
Co-authored-by: Harsha Reddy <harsha.reddy@klaviyo.com>
Co-authored-by: Zach H <zhirschtritt@gmail.com>
2024-11-04 10:06:14 +01:00

544 lines
11 KiB
Go

package database
import (
"context"
"database/sql"
"database/sql/driver"
"errors"
"fmt"
"regexp"
"testing"
"github.com/DATA-DOG/go-sqlmock"
"github.com/stretchr/testify/assert"
"github.com/zitadel/zitadel/internal/crypto"
z_db "github.com/zitadel/zitadel/internal/database"
db_mock "github.com/zitadel/zitadel/internal/database/mock"
"github.com/zitadel/zitadel/internal/zerrors"
)
func Test_database_ReadKeys(t *testing.T) {
type fields struct {
client db
masterKey string
decrypt func(encryptedKey, masterKey string) (key string, err error)
}
type res struct {
keys crypto.Keys
err func(error) bool
}
tests := []struct {
name string
fields fields
res res
}{
{
"query fails, error",
fields{
client: dbMock(t, expectQueryErr("SELECT id, key FROM system.encryption_keys", sql.ErrConnDone)),
masterKey: "",
decrypt: nil,
},
res{
err: func(err error) bool {
return errors.Is(err, sql.ErrConnDone)
},
},
},
{
"decryption error",
fields{
client: dbMock(t, expectQueryScanErr(
"SELECT id, key FROM system.encryption_keys",
[]string{"id", "key"},
[][]driver.Value{
{
"id1",
"key1",
},
})),
masterKey: "wrong key",
decrypt: func(encryptedKey, masterKey string) (key string, err error) {
return "", fmt.Errorf("wrong masterkey")
},
},
res{
err: zerrors.IsInternal,
},
},
{
"single key ok",
fields{
client: dbMock(t, expectQuery(
"SELECT id, key FROM system.encryption_keys",
[]string{"id", "key"},
[][]driver.Value{
{
"id1",
"key1",
},
})),
masterKey: "masterKey",
decrypt: func(encryptedKey, masterKey string) (key string, err error) {
return encryptedKey, nil
},
},
res{
keys: crypto.Keys(map[string]string{"id1": "key1"}),
},
},
{
"multiple keys ok",
fields{
client: dbMock(t, expectQuery(
"SELECT id, key FROM system.encryption_keys",
[]string{"id", "key"},
[][]driver.Value{
{
"id1",
"key1",
},
{
"id2",
"key2",
},
})),
masterKey: "masterKey",
decrypt: func(encryptedKey, masterKey string) (key string, err error) {
return encryptedKey, nil
},
},
res{
keys: crypto.Keys(map[string]string{"id1": "key1", "id2": "key2"}),
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := &Database{
client: tt.fields.client.db,
masterKey: tt.fields.masterKey,
decrypt: tt.fields.decrypt,
}
got, err := d.ReadKeys()
if tt.res.err == nil {
assert.NoError(t, err)
} else if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v", err)
}
if tt.res.err == nil {
assert.Equal(t, tt.res.keys, got)
}
if err := tt.fields.client.mock.ExpectationsWereMet(); err != nil {
t.Error(err)
}
})
}
}
func Test_database_ReadKey(t *testing.T) {
type fields struct {
client db
masterKey string
decrypt func(encryptedKey, masterKey string) (key string, err error)
}
type args struct {
id string
}
type res struct {
key *crypto.Key
err func(error) bool
}
tests := []struct {
name string
fields fields
args args
res res
}{
{
"query fails, error",
fields{
client: dbMock(t, expectQueryErr("SELECT key FROM system.encryption_keys WHERE id = $1", sql.ErrConnDone)),
masterKey: "",
decrypt: nil,
},
args{
id: "id1",
},
res{
err: func(err error) bool {
return errors.Is(err, sql.ErrConnDone)
},
},
},
{
"key not found err",
fields{
client: dbMock(t, expectQueryScanErr(
"SELECT key FROM system.encryption_keys WHERE id = $1",
nil,
nil,
"id1")),
masterKey: "masterKey",
decrypt: func(encryptedKey, masterKey string) (key string, err error) {
return encryptedKey, nil
},
},
args{
id: "id1",
},
res{
err: zerrors.IsInternal,
},
},
{
"decryption error",
fields{
client: dbMock(t, expectQueryScanErr(
"SELECT key FROM system.encryption_keys WHERE id = $1",
[]string{"key"},
[][]driver.Value{
{
"key1",
},
},
"id1",
)),
masterKey: "wrong key",
decrypt: func(encryptedKey, masterKey string) (key string, err error) {
return "", fmt.Errorf("wrong masterkey")
},
},
args{
id: "id1",
},
res{
err: zerrors.IsInternal,
},
},
{
"key ok",
fields{
client: dbMock(t, expectQuery(
"SELECT key FROM system.encryption_keys WHERE id = $1",
[]string{"key"},
[][]driver.Value{
{
"key1",
},
},
"id1",
)),
masterKey: "masterKey",
decrypt: func(encryptedKey, masterKey string) (key string, err error) {
return encryptedKey, nil
},
},
args{
id: "id1",
},
res{
key: &crypto.Key{
ID: "id1",
Value: "key1",
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := &Database{
client: tt.fields.client.db,
masterKey: tt.fields.masterKey,
decrypt: tt.fields.decrypt,
}
got, err := d.ReadKey(tt.args.id)
if tt.res.err == nil {
assert.NoError(t, err)
} else if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v", err)
}
if tt.res.err == nil {
assert.Equal(t, tt.res.key, got)
}
if err := tt.fields.client.mock.ExpectationsWereMet(); err != nil {
t.Error(err)
}
})
}
}
func Test_database_CreateKeys(t *testing.T) {
type fields struct {
client db
masterKey string
encrypt func(key, masterKey string) (encryptedKey string, err error)
}
type args struct {
keys []*crypto.Key
}
type res struct {
err func(error) bool
}
tests := []struct {
name string
fields fields
args args
res res
}{
{
"encryption fails, error",
fields{
client: dbMock(t),
masterKey: "",
encrypt: func(key, masterKey string) (encryptedKey string, err error) {
return "", fmt.Errorf("encryption failed")
},
},
args{
keys: []*crypto.Key{
{
"id1",
"key1",
},
},
},
res{
err: zerrors.IsInternal,
},
},
{
"insert fails, error",
fields{
client: dbMock(t,
expectBegin(nil),
expectExec("INSERT INTO system.encryption_keys (id,key) VALUES ($1,$2)", sql.ErrTxDone),
expectRollback(nil),
),
masterKey: "masterkey",
encrypt: func(key, masterKey string) (encryptedKey string, err error) {
return key, nil
},
},
args{
keys: []*crypto.Key{
{
"id1",
"key1",
},
},
},
res{
err: func(err error) bool {
return errors.Is(err, sql.ErrTxDone)
},
},
},
{
"single insert ok",
fields{
client: dbMock(t,
expectBegin(nil),
expectExec("INSERT INTO system.encryption_keys (id,key) VALUES ($1,$2)", nil, "id1", "key1"),
expectCommit(nil),
),
masterKey: "masterkey",
encrypt: func(key, masterKey string) (encryptedKey string, err error) {
return key, nil
},
},
args{
keys: []*crypto.Key{
{
"id1",
"key1",
},
},
},
res{
err: nil,
},
},
{
"multiple insert ok",
fields{
client: dbMock(t,
expectBegin(nil),
expectExec("INSERT INTO system.encryption_keys (id,key) VALUES ($1,$2)", nil, "id1", "key1", "id2", "key2"),
expectCommit(nil),
),
masterKey: "masterkey",
encrypt: func(key, masterKey string) (encryptedKey string, err error) {
return key, nil
},
},
args{
keys: []*crypto.Key{
{
"id1",
"key1",
},
{
"id2",
"key2",
},
},
},
res{
err: nil,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
d := &Database{
client: tt.fields.client.db,
masterKey: tt.fields.masterKey,
encrypt: tt.fields.encrypt,
}
err := d.CreateKeys(context.Background(), tt.args.keys...)
if tt.res.err == nil {
assert.NoError(t, err)
} else if tt.res.err != nil && !tt.res.err(err) {
t.Errorf("got wrong err: %v", err)
}
if err := tt.fields.client.mock.ExpectationsWereMet(); err != nil {
t.Error(err)
}
})
}
}
func Test_checkMasterKeyLength(t *testing.T) {
type args struct {
masterKey string
}
tests := []struct {
name string
args args
err func(error) bool
}{
{
"invalid length",
args{
masterKey: "",
},
zerrors.IsInternal,
},
{
"valid length",
args{
masterKey: "!themasterkeywhichis32byteslong!",
},
nil,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := checkMasterKeyLength(tt.args.masterKey)
if tt.err == nil {
assert.NoError(t, err)
} else if tt.err != nil && !tt.err(err) {
t.Errorf("got wrong err: %v", err)
}
})
}
}
type db struct {
mock sqlmock.Sqlmock
db *z_db.DB
}
func dbMock(t *testing.T, expectations ...func(m sqlmock.Sqlmock)) db {
t.Helper()
client, mock, err := sqlmock.New(sqlmock.ValueConverterOption(new(db_mock.TypeConverter)))
if err != nil {
t.Fatalf("unable to create sql mock: %v", err)
}
for _, expectation := range expectations {
expectation(mock)
}
return db{
mock: mock,
db: &z_db.DB{DB: client},
}
}
func expectQueryErr(query string, err error, args ...driver.Value) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
m.ExpectQuery(regexp.QuoteMeta(query)).WithArgs(args...).WillReturnError(err)
}
}
func expectQueryScanErr(stmt string, cols []string, rows [][]driver.Value, args ...driver.Value) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
q := m.ExpectQuery(regexp.QuoteMeta(stmt)).WithArgs(args...)
result := m.NewRows(cols)
count := uint64(len(rows))
for _, row := range rows {
if cols[len(cols)-1] == "count" {
row = append(row, count)
}
result.AddRow(row...)
}
q.WillReturnRows(result)
q.RowsWillBeClosed()
}
}
func expectQuery(stmt string, cols []string, rows [][]driver.Value, args ...driver.Value) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
q := m.ExpectQuery(regexp.QuoteMeta(stmt)).WithArgs(args...)
result := m.NewRows(cols)
count := uint64(len(rows))
for _, row := range rows {
if cols[len(cols)-1] == "count" {
row = append(row, count)
}
result.AddRow(row...)
}
q.WillReturnRows(result)
q.RowsWillBeClosed()
}
}
func expectExec(stmt string, err error, args ...driver.Value) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
query := m.ExpectExec(regexp.QuoteMeta(stmt)).WithArgs(args...)
if err != nil {
query.WillReturnError(err)
return
}
query.WillReturnResult(sqlmock.NewResult(1, 1))
}
}
func expectBegin(err error) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
query := m.ExpectBegin()
if err != nil {
query.WillReturnError(err)
}
}
}
func expectCommit(err error) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
query := m.ExpectCommit()
if err != nil {
query.WillReturnError(err)
}
}
}
func expectRollback(err error) func(m sqlmock.Sqlmock) {
return func(m sqlmock.Sqlmock) {
query := m.ExpectRollback()
if err != nil {
query.WillReturnError(err)
}
}
}