Skip to content

Commit f899bd7

Browse files
dave-tuckerdave-tucker
committed
cache: Disable Index Checking on Populate
The ordering of the updates from the table-update notification is undefined. It's a hard problem to sort the update in such a way each operation would be processed in order to ensure no indexes collided. We can instead take the easy way out and ignore index checking altogether as we assume the server has already take care of this. THe cache is locked during update, so the client doesn't need to worry about the indexes being inconsistent during the populate. Signed-off-by: Dave Tucker <[email protected]>
1 parent fecdcab commit f899bd7

File tree

3 files changed

+58
-26
lines changed

3 files changed

+58
-26
lines changed

‎cache/cache.go

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ func (r *RowCache) RowByModel(m model.Model) model.Model {
114114
}
115115

116116
// Create writes the provided content to the cache
117-
func (r *RowCache) Create(uuid string, m model.Model) error {
117+
func (r *RowCache) Create(uuid string, m model.Model, checkIndexes bool) error {
118118
r.mutex.Lock()
119119
defer r.mutex.Unlock()
120120
if _, ok := r.cache[uuid]; ok {
@@ -129,15 +129,15 @@ func (r *RowCache) Create(uuid string, m model.Model) error {
129129
}
130130
newIndexes := newColumnToValue(r.schema.Indexes)
131131
for index := range r.indexes {
132-
133132
val, err := valueFromIndex(info, index)
134-
135133
if err != nil {
136134
return err
137135
}
138-
if existing, ok := r.indexes[index][val]; ok {
136+
137+
if existing, ok := r.indexes[index][val]; ok && checkIndexes {
139138
return NewIndexExistsError(r.name, val, index, uuid, existing)
140139
}
140+
141141
newIndexes[index][val] = uuid
142142
}
143143

@@ -152,7 +152,7 @@ func (r *RowCache) Create(uuid string, m model.Model) error {
152152
}
153153

154154
// Update updates the content in the cache
155-
func (r *RowCache) Update(uuid string, m model.Model) error {
155+
func (r *RowCache) Update(uuid string, m model.Model, checkIndexes bool) error {
156156
r.mutex.Lock()
157157
defer r.mutex.Unlock()
158158
if _, ok := r.cache[uuid]; !ok {
@@ -188,7 +188,8 @@ func (r *RowCache) Update(uuid string, m model.Model) error {
188188
// old and new values are NOT the same
189189

190190
// check that there are no conflicts
191-
if conflict, ok := r.indexes[index][newVal]; ok && conflict != uuid {
191+
192+
if conflict, ok := r.indexes[index][newVal]; ok && checkIndexes && conflict != uuid {
192193
errs = append(errs, NewIndexExistsError(
193194
r.name,
194195
newVal,
@@ -197,6 +198,7 @@ func (r *RowCache) Update(uuid string, m model.Model) error {
197198
conflict,
198199
))
199200
}
201+
200202
newIndexes[index][newVal] = uuid
201203
oldIndexes[index][oldVal] = ""
202204
}
@@ -339,7 +341,7 @@ func NewTableCache(schema *ovsdb.DatabaseSchema, dbModel *model.DBModel, data Da
339341
return nil, fmt.Errorf("table %s is not in schema", table)
340342
}
341343
for uuid, row := range rowData {
342-
if err := cache[table].Create(uuid, row); err != nil {
344+
if err := cache[table].Create(uuid, row, true); err != nil {
343345
return nil, err
344346
}
345347
}
@@ -414,6 +416,7 @@ func (t *TableCache) Disconnected() {
414416
func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) {
415417
t.mutex.Lock()
416418
defer t.mutex.Unlock()
419+
417420
for table := range t.dbModel.Types() {
418421
updates, ok := tableUpdates[table]
419422
if !ok {
@@ -428,15 +431,15 @@ func (t *TableCache) Populate(tableUpdates ovsdb.TableUpdates) {
428431
}
429432
if existing := tCache.Row(uuid); existing != nil {
430433
if !reflect.DeepEqual(newModel, existing) {
431-
if err := tCache.Update(uuid, newModel); err != nil {
434+
if err := tCache.Update(uuid, newModel, false); err != nil {
432435
panic(err)
433436
}
434437
t.eventProcessor.AddEvent(updateEvent, table, existing, newModel)
435438
}
436439
// no diff
437440
continue
438441
}
439-
if err := tCache.Create(uuid, newModel); err != nil {
442+
if err := tCache.Create(uuid, newModel, false); err != nil {
440443
panic(err)
441444
}
442445
t.eventProcessor.AddEvent(addEvent, table, nil, newModel)

‎cache/cache_test.go

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -111,35 +111,53 @@ func TestRowCacheCreate(t *testing.T) {
111111
require.Nil(t, err)
112112

113113
tests := []struct {
114-
name string
115-
uuid string
116-
model *testModel
117-
wantErr bool
114+
name string
115+
uuid string
116+
model *testModel
117+
checkIndex bool
118+
wantErr bool
118119
}{
119120
{
120121
"inserts a new row",
121122
"foo",
122123
&testModel{Foo: "foo"},
124+
true,
123125
false,
124126
},
125127
{
126128
"error duplicate uuid",
127129
"bar",
128130
&testModel{Foo: "foo"},
129131
true,
132+
true,
130133
},
131134
{
132135
"error duplicate index",
133136
"baz",
134137
&testModel{Foo: "bar"},
135138
true,
139+
true,
140+
},
141+
{
142+
"error duplicate uuid, no index check",
143+
"bar",
144+
&testModel{Foo: "bar"},
145+
false,
146+
true,
147+
},
148+
{
149+
"no error duplicate index",
150+
"baz",
151+
&testModel{Foo: "bar"},
152+
false,
153+
false,
136154
},
137155
}
138156
for _, tt := range tests {
139157
t.Run(tt.name, func(t *testing.T) {
140158
rc := tc.Table("Open_vSwitch")
141159
require.NotNil(t, rc)
142-
err := rc.Create(tt.uuid, tt.model)
160+
err := rc.Create(tt.uuid, tt.model, tt.checkIndex)
143161
if tt.wantErr {
144162
assert.Error(t, err)
145163
} else {
@@ -225,7 +243,7 @@ func TestRowCacheCreateMultiIndex(t *testing.T) {
225243
t.Run(tt.name, func(t *testing.T) {
226244
rc := tc.Table("Open_vSwitch")
227245
require.NotNil(t, rc)
228-
err := rc.Create(tt.uuid, tt.model)
246+
err := rc.Create(tt.uuid, tt.model, true)
229247
if tt.wantErr {
230248
assert.Error(t, err)
231249
if tt.wantIndexExistsErr {
@@ -275,35 +293,46 @@ func TestRowCacheUpdate(t *testing.T) {
275293
require.Nil(t, err)
276294

277295
tests := []struct {
278-
name string
279-
uuid string
280-
model *testModel
281-
wantErr bool
296+
name string
297+
uuid string
298+
model *testModel
299+
checkIndex bool
300+
wantErr bool
282301
}{
283302
{
284303
"error if row does not exist",
285304
"foo",
286305
&testModel{Foo: "foo"},
287306
true,
307+
true,
288308
},
289309
{
290310
"update",
291311
"bar",
292312
&testModel{Foo: "baz"},
313+
true,
293314
false,
294315
},
295316
{
296317
"error new index would cause duplicate",
297-
"baz",
318+
"bar",
298319
&testModel{Foo: "foobar"},
299320
true,
321+
true,
322+
},
323+
{
324+
"no error new index would cause duplicate",
325+
"bar",
326+
&testModel{Foo: "foobar"},
327+
false,
328+
false,
300329
},
301330
}
302331
for _, tt := range tests {
303332
t.Run(tt.name, func(t *testing.T) {
304333
rc := tc.Table("Open_vSwitch")
305334
require.NotNil(t, rc)
306-
err := rc.Update(tt.uuid, tt.model)
335+
err := rc.Update(tt.uuid, tt.model, tt.checkIndex)
307336
if tt.wantErr {
308337
assert.Error(t, err)
309338
} else {
@@ -381,7 +410,7 @@ func TestRowCacheUpdateMultiIndex(t *testing.T) {
381410
t.Run(tt.name, func(t *testing.T) {
382411
rc := tc.Table("Open_vSwitch")
383412
require.NotNil(t, rc)
384-
err := rc.Update(tt.uuid, tt.model)
413+
err := rc.Update(tt.uuid, tt.model, true)
385414
if tt.wantErr {
386415
assert.Error(t, err)
387416
} else {
@@ -792,7 +821,7 @@ func TestIndex(t *testing.T) {
792821
}
793822
table := tc.Table("Open_vSwitch")
794823

795-
err = table.Create(obj.UUID, obj)
824+
err = table.Create(obj.UUID, obj, true)
796825
assert.Nil(t, err)
797826
t.Run("Index by single column", func(t *testing.T) {
798827
idx, err := table.Index("foo")

‎server/database.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ func (db *inMemoryDatabase) Insert(database string, table string, rowUUID string
142142
}
143143

144144
// insert in to db
145-
if err := targetDb.Table(table).Create(rowUUID, model); err != nil {
145+
if err := targetDb.Table(table).Create(rowUUID, model, true); err != nil {
146146
if indexErr, ok := err.(*cache.IndexExistsError); ok {
147147
e := ovsdb.ConstraintViolation{}
148148
return ovsdb.OperationResult{
@@ -227,7 +227,7 @@ func (db *inMemoryDatabase) Update(database, table string, where []ovsdb.Conditi
227227
if err != nil {
228228
panic(err)
229229
}
230-
if err = targetDb.Table(table).Update(uuid.(string), row); err != nil {
230+
if err = targetDb.Table(table).Update(uuid.(string), row, true); err != nil {
231231
if indexErr, ok := err.(*cache.IndexExistsError); ok {
232232
e := ovsdb.ConstraintViolation{}
233233
return ovsdb.OperationResult{
@@ -300,7 +300,7 @@ func (db *inMemoryDatabase) Mutate(database, table string, where []ovsdb.Conditi
300300
panic(err)
301301
}
302302
// the field in old has been set, write back to db
303-
err = targetDb.Table(table).Update(uuid.(string), old)
303+
err = targetDb.Table(table).Update(uuid.(string), old, true)
304304
if err != nil {
305305
if indexErr, ok := err.(*cache.IndexExistsError); ok {
306306
e := ovsdb.ConstraintViolation{}

0 commit comments

Comments
 (0)