diff --git a/weed/filer/mysql/mysql_sql_gen.go b/weed/filer/mysql/mysql_sql_gen.go index a2e07002b..8e77bab8a 100644 --- a/weed/filer/mysql/mysql_sql_gen.go +++ b/weed/filer/mysql/mysql_sql_gen.go @@ -13,6 +13,13 @@ type SqlGenMysql struct { UpsertQueryTemplate string } +// DefaultUpsertQuery keeps INSERTs idempotent so the inode-index KvPut +// after every entry write does not emit a duplicate-key roundtrip on +// every mutation. The VALUES() form works on MariaDB and MySQL >=5.7; +// the newer "AS new" alias from MySQL 8.0.19 errors on MariaDB, so it +// is left for users to opt into via an explicit upsertQuery. +const DefaultUpsertQuery = "INSERT INTO `%s` (`dirhash`,`name`,`directory`,`meta`) VALUES(?,?,?,?) ON DUPLICATE KEY UPDATE `meta` = VALUES(`meta`)" + var ( _ = abstract_sql.SqlGenerator(&SqlGenMysql{}) ) diff --git a/weed/filer/mysql/mysql_sql_gen_test.go b/weed/filer/mysql/mysql_sql_gen_test.go new file mode 100644 index 000000000..74303b764 --- /dev/null +++ b/weed/filer/mysql/mysql_sql_gen_test.go @@ -0,0 +1,28 @@ +package mysql + +import ( + "strings" + "testing" +) + +func TestDefaultUpsertQueryUsesOnDuplicateKey(t *testing.T) { + gen := &SqlGenMysql{UpsertQueryTemplate: DefaultUpsertQuery} + got := gen.GetSqlInsert("filemeta") + if !strings.Contains(got, "ON DUPLICATE KEY UPDATE") { + t.Fatalf("expected ON DUPLICATE KEY UPDATE in default upsert, got: %s", got) + } + if strings.Contains(got, "AS `new`") { + t.Fatalf("default should avoid MySQL 8.0.19 row-alias syntax for MariaDB compat, got: %s", got) + } + if !strings.Contains(got, "`filemeta`") { + t.Fatalf("expected backticked table name, got: %s", got) + } +} + +func TestEmptyUpsertTemplateFallsBackToPlainInsert(t *testing.T) { + gen := &SqlGenMysql{} + got := gen.GetSqlInsert("filemeta") + if strings.Contains(got, "ON DUPLICATE KEY UPDATE") { + t.Fatalf("plain INSERT path should not contain ON DUPLICATE KEY UPDATE, got: %s", got) + } +} diff --git a/weed/filer/mysql/mysql_store.go b/weed/filer/mysql/mysql_store.go index 2e83e7a4d..b5b5ef025 100644 --- a/weed/filer/mysql/mysql_store.go +++ b/weed/filer/mysql/mysql_store.go @@ -35,6 +35,9 @@ func (store *MysqlStore) GetName() string { func (store *MysqlStore) Initialize(configuration util.Configuration, prefix string) (err error) { // Absent key keeps a pooled default; an explicit 0 disables the idle pool. configuration.SetDefault(prefix+"connection_max_idle", 2) + // Default on so minimal configs avoid the duplicate-key roundtrip the + // inode-index KvPut would otherwise emit on every write. + configuration.SetDefault(prefix+"enableUpsert", true) return store.initialize( configuration.GetString(prefix+"dsn"), configuration.GetString(prefix+"upsertQuery"), @@ -64,6 +67,8 @@ func (store *MysqlStore) initialize(dsn string, upsertQuery string, enableUpsert store.SupportBucketTable = false if !enableUpsert { upsertQuery = "" + } else if upsertQuery == "" { + upsertQuery = DefaultUpsertQuery } store.SqlGenerator = &SqlGenMysql{ CreateTableSqlTemplate: "", diff --git a/weed/filer/mysql2/mysql2_store.go b/weed/filer/mysql2/mysql2_store.go index 82d0c86c2..3dde672b0 100644 --- a/weed/filer/mysql2/mysql2_store.go +++ b/weed/filer/mysql2/mysql2_store.go @@ -35,6 +35,9 @@ func (store *MysqlStore2) GetName() string { func (store *MysqlStore2) Initialize(configuration util.Configuration, prefix string) (err error) { // Absent key keeps a pooled default; an explicit 0 disables the idle pool. configuration.SetDefault(prefix+"connection_max_idle", 2) + // Default on so minimal configs avoid the duplicate-key roundtrip the + // inode-index KvPut would otherwise emit on every write. + configuration.SetDefault(prefix+"enableUpsert", true) return store.initialize( configuration.GetString(prefix+"createTable"), configuration.GetString(prefix+"upsertQuery"), @@ -57,6 +60,8 @@ func (store *MysqlStore2) initialize(createTable, upsertQuery string, enableUpse store.SupportBucketTable = true if !enableUpsert { upsertQuery = "" + } else if upsertQuery == "" { + upsertQuery = mysql.DefaultUpsertQuery } store.SqlGenerator = &mysql.SqlGenMysql{ CreateTableSqlTemplate: createTable, diff --git a/weed/filer/postgres/postgres_sql_gen.go b/weed/filer/postgres/postgres_sql_gen.go index 8832e1a45..89696d810 100644 --- a/weed/filer/postgres/postgres_sql_gen.go +++ b/weed/filer/postgres/postgres_sql_gen.go @@ -13,6 +13,11 @@ type SqlGenPostgres struct { UpsertQueryTemplate string } +// DefaultUpsertQuery keeps INSERTs idempotent so a duplicate-key failure +// (23505) cannot poison the surrounding transaction (25P02). Used when the +// user enables upsert but does not provide their own template. +const DefaultUpsertQuery = `INSERT INTO "%s" (dirhash,name,directory,meta) VALUES($1,$2,$3,$4) ON CONFLICT (dirhash, name) DO UPDATE SET directory=EXCLUDED.directory, meta=EXCLUDED.meta` + var ( _ = abstract_sql.SqlGenerator(&SqlGenPostgres{}) ) diff --git a/weed/filer/postgres/postgres_sql_gen_test.go b/weed/filer/postgres/postgres_sql_gen_test.go new file mode 100644 index 000000000..16039803c --- /dev/null +++ b/weed/filer/postgres/postgres_sql_gen_test.go @@ -0,0 +1,25 @@ +package postgres + +import ( + "strings" + "testing" +) + +func TestDefaultUpsertQueryIsConflictSafe(t *testing.T) { + gen := &SqlGenPostgres{UpsertQueryTemplate: DefaultUpsertQuery} + got := gen.GetSqlInsert("filemeta") + if !strings.Contains(got, "ON CONFLICT") { + t.Fatalf("expected ON CONFLICT in default upsert, got: %s", got) + } + if !strings.Contains(got, `"filemeta"`) { + t.Fatalf("expected quoted table name, got: %s", got) + } +} + +func TestEmptyUpsertTemplateFallsBackToPlainInsert(t *testing.T) { + gen := &SqlGenPostgres{} + got := gen.GetSqlInsert("filemeta") + if strings.Contains(got, "ON CONFLICT") { + t.Fatalf("plain INSERT path should not contain ON CONFLICT, got: %s", got) + } +} diff --git a/weed/filer/postgres/postgres_store.go b/weed/filer/postgres/postgres_store.go index fa6cb7df6..97f90b83f 100644 --- a/weed/filer/postgres/postgres_store.go +++ b/weed/filer/postgres/postgres_store.go @@ -30,6 +30,9 @@ func (store *PostgresStore) GetName() string { func (store *PostgresStore) Initialize(configuration util.Configuration, prefix string) (err error) { // Absent key keeps a pooled default; an explicit 0 disables the idle pool. configuration.SetDefault(prefix+"connection_max_idle", 2) + // Default on so minimal configs are not exposed to duplicate-key tx + // poisoning on Postgres; an explicit false still disables it. + configuration.SetDefault(prefix+"enableUpsert", true) return store.initialize( configuration.GetString(prefix+"upsertQuery"), configuration.GetBool(prefix+"enableUpsert"), @@ -56,6 +59,8 @@ func (store *PostgresStore) initialize(upsertQuery string, enableUpsert bool, us store.SupportBucketTable = false if !enableUpsert { upsertQuery = "" + } else if upsertQuery == "" { + upsertQuery = DefaultUpsertQuery } store.SqlGenerator = &SqlGenPostgres{ CreateTableSqlTemplate: "", diff --git a/weed/filer/postgres2/postgres2_store.go b/weed/filer/postgres2/postgres2_store.go index 64e86b9da..3861e0689 100644 --- a/weed/filer/postgres2/postgres2_store.go +++ b/weed/filer/postgres2/postgres2_store.go @@ -35,6 +35,9 @@ func (store *PostgresStore2) GetName() string { func (store *PostgresStore2) Initialize(configuration util.Configuration, prefix string) (err error) { // Absent key keeps a pooled default; an explicit 0 disables the idle pool. configuration.SetDefault(prefix+"connection_max_idle", 2) + // Default on so minimal configs are not exposed to duplicate-key tx + // poisoning on Postgres; an explicit false still disables it. + configuration.SetDefault(prefix+"enableUpsert", true) return store.initialize( configuration.GetString(prefix+"createTable"), configuration.GetString(prefix+"upsertQuery"), @@ -62,6 +65,8 @@ func (store *PostgresStore2) initialize(createTable, upsertQuery string, enableU store.SupportBucketTable = true if !enableUpsert { upsertQuery = "" + } else if upsertQuery == "" { + upsertQuery = postgres.DefaultUpsertQuery } store.SqlGenerator = &postgres.SqlGenPostgres{ CreateTableSqlTemplate: createTable,