diff --git a/config/database.go b/config/database.go index f0bdc1a657..982214eb30 100644 --- a/config/database.go +++ b/config/database.go @@ -6,8 +6,10 @@ package config import ( "bytes" "context" + "crypto/sha256" "database/sql" "embed" + "encoding/hex" "encoding/json" "fmt" "path/filepath" @@ -219,48 +221,81 @@ func (ds *DatabaseStore) persist(cfg *model.Config) error { return errors.Wrap(err, "failed to serialize") } - id := model.NewId() value := string(b) - createAt := model.GetMillis() - err = ds.checkLength(len(value)) if err != nil { return errors.Wrap(err, "marshalled configuration failed length check") } + sum := sha256.Sum256(b) + + // Skip the persist altogether if we're effectively writing the same configuration. + var oldValue string + var row *sql.Row + if ds.driverName == model.DatabaseDriverMysql { + // We use a sub-query to get the Id first because selecting the Id column using + // active uses the index, but selecting SHA column using active does not use the index. + // The sub-query uses the active index, and then the top-level query uses the primary key. + // This takes 2 queries, but it is actually faster than one slow query for MySQL + row = ds.db.QueryRow("SELECT SHA FROM Configurations WHERE Id = (select Id from Configurations Where Active)") + } else { + row = ds.db.QueryRow("SELECT SHA FROM Configurations WHERE Active") + } + if err = row.Scan(&oldValue); err != nil && err != sql.ErrNoRows { + return errors.Wrap(err, "failed to query active configuration") + } + + // postgres retruns blank-padded therefore we trim the space + oldSum, err := hex.DecodeString(strings.TrimSpace(oldValue)) + if err != nil { + return errors.Wrap(err, "could not encode value") + } + + // compare checksums, it's more efficient rather than comparing entire config itself + if bytes.Equal(oldSum, sum[0:]) { + return nil + } + tx, err := ds.db.Beginx() if err != nil { return errors.Wrap(err, "failed to begin transaction") } defer func() { // Rollback after Commit just returns sql.ErrTxDone. - if err := tx.Rollback(); err != nil && err != sql.ErrTxDone { + if err = tx.Rollback(); err != nil && err != sql.ErrTxDone { mlog.Error("Failed to rollback configuration transaction", mlog.Err(err)) } }() + var oldId string + if ds.driverName == model.DatabaseDriverMysql { + // the query doesn't use active index if we query for value (mysql, no surprise) + // we select Id column which triggers using index hence we do quicker reads + // that's the reason we select id first then query against id to get the value. + row = tx.QueryRow("SELECT Id FROM Configurations WHERE Active") + if err = row.Scan(&oldId); err != nil && err != sql.ErrNoRows { + return errors.Wrap(err, "failed to query active configuration") + } + if oldId != "" { + if _, err := tx.NamedExec("UPDATE Configurations SET Active = NULL WHERE Id = :id", map[string]interface{}{"id": oldId}); err != nil { + return errors.Wrap(err, "failed to deactivate current configuration") + } + } + } else { + if _, err := tx.Exec("UPDATE Configurations SET Active = NULL WHERE Active"); err != nil { + return errors.Wrap(err, "failed to deactivate current configuration") + } + } + params := map[string]interface{}{ - "id": id, + "id": model.NewId(), "value": value, - "create_at": createAt, + "create_at": model.GetMillis(), "key": "ConfigurationId", + "sha": hex.EncodeToString(sum[0:]), } - // Skip the persist altogether if we're effectively writing the same configuration. - var oldValue []byte - row := ds.db.QueryRow("SELECT Value FROM Configurations WHERE Active") - if err := row.Scan(&oldValue); err != nil && err != sql.ErrNoRows { - return errors.Wrap(err, "failed to query active configuration") - } - if bytes.Equal(oldValue, b) { - return nil - } - - if _, err := tx.Exec("UPDATE Configurations SET Active = NULL WHERE Active"); err != nil { - return errors.Wrap(err, "failed to deactivate current configuration") - } - - if _, err := tx.NamedExec("INSERT INTO Configurations (Id, Value, CreateAt, Active) VALUES (:id, :value, :create_at, TRUE)", params); err != nil { + if _, err := tx.NamedExec("INSERT INTO Configurations (Id, Value, CreateAt, Active, SHA) VALUES (:id, :value, :create_at, TRUE, :sha)", params); err != nil { return errors.Wrap(err, "failed to record new configuration") } diff --git a/config/migrations/mysql/000003_update_configurations_sha.down.sql b/config/migrations/mysql/000003_update_configurations_sha.down.sql new file mode 100644 index 0000000000..f53330f258 --- /dev/null +++ b/config/migrations/mysql/000003_update_configurations_sha.down.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'Configurations' + AND table_schema = DATABASE() + AND column_name = 'SHA' + ) > 0, + 'ALTER TABLE Configurations DROP COLUMN SHA;', + 'SELECT 1' +)); + +PREPARE alterIfExists FROM @preparedStatement; +EXECUTE alterIfExists; +DEALLOCATE PREPARE alterIfExists; diff --git a/config/migrations/mysql/000003_update_configurations_sha.up.sql b/config/migrations/mysql/000003_update_configurations_sha.up.sql new file mode 100644 index 0000000000..120dd86e2e --- /dev/null +++ b/config/migrations/mysql/000003_update_configurations_sha.up.sql @@ -0,0 +1,14 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'Configurations' + AND table_schema = DATABASE() + AND column_name = 'SHA' + ) > 0, + 'SELECT 1', + 'ALTER TABLE Configurations ADD COLUMN SHA char(64) DEFAULT "";' +)); + +PREPARE alterIfNotExists FROM @preparedStatement; +EXECUTE alterIfNotExists; +DEALLOCATE PREPARE alterIfNotExists; diff --git a/config/migrations/postgres/000003_update_configurations_sha.down.sql b/config/migrations/postgres/000003_update_configurations_sha.down.sql new file mode 100644 index 0000000000..b49bf928cf --- /dev/null +++ b/config/migrations/postgres/000003_update_configurations_sha.down.sql @@ -0,0 +1 @@ +ALTER TABLE Configurations DROP COLUMN IF EXISTS SHA; diff --git a/config/migrations/postgres/000003_update_configurations_sha.up.sql b/config/migrations/postgres/000003_update_configurations_sha.up.sql new file mode 100644 index 0000000000..86a8b4bd9f --- /dev/null +++ b/config/migrations/postgres/000003_update_configurations_sha.up.sql @@ -0,0 +1 @@ +ALTER TABLE Configurations ADD COLUMN IF NOT EXISTS SHA CHAR(64) DEFAULT '';