fix(filer/postgres): default to ON CONFLICT upsert to keep tx alive (#9709)

* fix(filer/postgres): default to ON CONFLICT upsert to keep tx alive

A KvPut from the inode-index secondary write could fail with 23505
(duplicate key) inside a rename's transaction, after which the next
statement returned 25P02 and rename surfaced to FUSE as EIO. Default
the postgres upsert query when enableUpsert=true so INSERTs are
idempotent; the enableUpsert=false escape hatch is preserved for
non-PG-compatible backends.

* fix(filer/mysql): default to ON DUPLICATE KEY UPDATE upsert

Same shape as the postgres default: when enableUpsert=true but no
upsertQuery is configured, install a sensible default so the
inode-index KvPut does not waste a duplicate-key roundtrip on every
entry write. Uses the VALUES() form so the default works on MariaDB
and MySQL >=5.7; the MySQL 8.0.19 row-alias form is left to explicit
config.

* fix(filer): default enableUpsert=true for sql stores

The default-template fallback only kicks in when enableUpsert=true,
so minimal configs that omit the flag entirely were still exposed.
Default it on for postgres/postgres2/mysql/mysql2; an explicit false
in filer.toml still wins because SetDefault only fills absent keys.
This commit is contained in:
Chris Lu
2026-05-27 12:23:30 -07:00
committed by GitHub
parent 396e3c326b
commit 21b4b81edb
8 changed files with 85 additions and 0 deletions
+7
View File
@@ -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{})
)
+28
View File
@@ -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)
}
}
+5
View File
@@ -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: "",
+5
View File
@@ -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,
+5
View File
@@ -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{})
)
@@ -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)
}
}
+5
View File
@@ -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: "",
+5
View File
@@ -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,