Fixing key value duplication race condition in postgres (#13753)
Automatic Merge
Этот коммит содержится в:
коммит произвёл
GitHub
родитель
acce0da068
Коммит
7afafd7767
@@ -63,7 +63,7 @@ func (ps SqlPluginStore) SaveOrUpdate(kv *model.PluginKeyValue) (*model.PluginKe
|
|||||||
// If the error is from unique constraints violation, it's the result of a
|
// If the error is from unique constraints violation, it's the result of a
|
||||||
// valid race and we can report success. Otherwise we have a real error and
|
// valid race and we can report success. Otherwise we have a real error and
|
||||||
// need to return it
|
// need to return it
|
||||||
if !IsUniqueConstraintError(err, []string{"PRIMARY", "PluginId", "Key", "PKey"}) {
|
if !IsUniqueConstraintError(err, []string{"PRIMARY", "PluginId", "Key", "PKey", "pkey"}) {
|
||||||
return nil, model.NewAppError("SqlPluginStore.SaveOrUpdate", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
|
return nil, model.NewAppError("SqlPluginStore.SaveOrUpdate", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -93,7 +93,7 @@ func (ps SqlPluginStore) CompareAndSet(kv *model.PluginKeyValue, oldValue []byte
|
|||||||
// If the error is from unique constraints violation, it's the result of a
|
// If the error is from unique constraints violation, it's the result of a
|
||||||
// race condition, return false and no error. Otherwise we have a real error and
|
// race condition, return false and no error. Otherwise we have a real error and
|
||||||
// need to return it.
|
// need to return it.
|
||||||
if IsUniqueConstraintError(err, []string{"PRIMARY", "PluginId", "Key", "PKey"}) {
|
if IsUniqueConstraintError(err, []string{"PRIMARY", "PluginId", "Key", "PKey", "pkey"}) {
|
||||||
return false, nil
|
return false, nil
|
||||||
} else {
|
} else {
|
||||||
return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
|
return false, model.NewAppError("SqlPluginStore.CompareAndSet", "store.sql_plugin_store.save.app_error", nil, err.Error(), http.StatusInternalServerError)
|
||||||
|
|||||||
@@ -39,6 +39,22 @@ func testPluginCompareAndSet(t *testing.T, ss store.Store) {
|
|||||||
assert.True(t, ok)
|
assert.True(t, ok)
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("set existing key without old value should fail without error because is a automatically handled race condition", func(t *testing.T) {
|
||||||
|
_, err := ss.Plugin().SaveOrUpdate(kv)
|
||||||
|
require.Nil(t, err)
|
||||||
|
|
||||||
|
kvNew := &model.PluginKeyValue{
|
||||||
|
PluginId: kv.PluginId,
|
||||||
|
Key: kv.Key,
|
||||||
|
Value: []byte(model.NewId()),
|
||||||
|
ExpireAt: 0,
|
||||||
|
}
|
||||||
|
|
||||||
|
ok, err := ss.Plugin().CompareAndSet(kvNew, nil)
|
||||||
|
require.Nil(t, err)
|
||||||
|
assert.False(t, ok)
|
||||||
|
})
|
||||||
|
|
||||||
t.Run("set existing key with new value should succeed given same old value", func(t *testing.T) {
|
t.Run("set existing key with new value should succeed given same old value", func(t *testing.T) {
|
||||||
_, err := ss.Plugin().SaveOrUpdate(kv)
|
_, err := ss.Plugin().SaveOrUpdate(kv)
|
||||||
require.Nil(t, err)
|
require.Nil(t, err)
|
||||||
|
|||||||
Ссылка в новой задаче
Block a user