Skip to content

Commit

Permalink
sharedobjcat: allow addition/deletion of object in same batch
Browse files Browse the repository at this point in the history
On rare occasions, if we cancel a compaction at a point right after
creating a new object, we'll end up deleting the object shortly after.
Sometimes the two can happen with no explicit Sync() call in between,
resulting in the object being created and deleted in the same batch
which currently results in an error. This change allows for this case
by moving around logic in the catalog batch application code.
  • Loading branch information
itsbilal committed Jul 10, 2023
1 parent 52a12cd commit a9a079d
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 26 deletions.
30 changes: 19 additions & 11 deletions objstorage/objstorageprovider/sharedobjcat/catalog.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,28 +218,36 @@ func (c *Catalog) ApplyBatch(b Batch) error {
c.mu.Lock()
defer c.mu.Unlock()

// Add new objects before deleting any objects. This allows for cases where
// the same batch adds and deletes an object.
for _, meta := range b.ve.NewObjects {
if _, exists := c.mu.objects[meta.FileNum]; exists {
return errors.AssertionFailedf("adding existing object %s", meta.FileNum)
}
}
for _, meta := range b.ve.NewObjects {
c.mu.objects[meta.FileNum] = meta
}
removeAddedObjects := func() {
for i := range b.ve.NewObjects {
delete(c.mu.objects, b.ve.NewObjects[i].FileNum)
}
}
for _, n := range b.ve.DeletedObjects {
if _, exists := c.mu.objects[n]; !exists {
removeAddedObjects()
return errors.AssertionFailedf("deleting non-existent object %s", n)
}
}
for _, meta := range b.ve.NewObjects {
if _, exists := c.mu.objects[meta.FileNum]; exists {
return errors.AssertionFailedf("adding existing object %s", meta.FileNum)
}
// Apply the remainder of the batch to our current state.
for _, n := range b.ve.DeletedObjects {
delete(c.mu.objects, n)
}

if err := c.writeToCatalogFileLocked(&b.ve); err != nil {
return errors.Wrapf(err, "pebble: could not write to shared object catalog: %v", err)
}

// Apply the batch to our current state.
for _, n := range b.ve.DeletedObjects {
delete(c.mu.objects, n)
}
for _, meta := range b.ve.NewObjects {
c.mu.objects[meta.FileNum] = meta
}
b.Reset()
return nil
}
Expand Down
30 changes: 15 additions & 15 deletions objstorage/objstorageprovider/sharedobjcat/testdata/catalog
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,14 @@ close: other-path/marker.shared-catalog.000001.SHARED-CATALOG-000001
sync: other-path
sync: other-path/SHARED-CATALOG-000001

# Adding and deleting objects in the same batch is allowed.

batch
add 9 50 501
delete 9
----
sync: other-path/SHARED-CATALOG-000001

list other-path
----
SHARED-CATALOG-000001
Expand Down Expand Up @@ -169,7 +177,6 @@ sync: test/SHARED-CATALOG-000003
sync: test/SHARED-CATALOG-000003
sync: test/SHARED-CATALOG-000003
sync: test/SHARED-CATALOG-000003
sync: test/SHARED-CATALOG-000003
close: test/SHARED-CATALOG-000003
create: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
Expand All @@ -181,6 +188,7 @@ remove: test/SHARED-CATALOG-000003
sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004

list test
----
Expand All @@ -202,7 +210,6 @@ sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
sync: test/SHARED-CATALOG-000004
close: test/SHARED-CATALOG-000004
create: test/SHARED-CATALOG-000005
sync: test/SHARED-CATALOG-000005
Expand All @@ -217,6 +224,7 @@ sync: test/SHARED-CATALOG-000005
sync: test/SHARED-CATALOG-000005
sync: test/SHARED-CATALOG-000005
sync: test/SHARED-CATALOG-000005
sync: test/SHARED-CATALOG-000005

list test
----
Expand All @@ -237,6 +245,7 @@ sync: test
remove: test/SHARED-CATALOG-000005
sync: test/SHARED-CATALOG-000006
sync: test/SHARED-CATALOG-000006
sync: test/SHARED-CATALOG-000006
close: test/SHARED-CATALOG-000006
create: test/SHARED-CATALOG-000007
sync: test/SHARED-CATALOG-000007
Expand All @@ -249,19 +258,10 @@ sync: test/SHARED-CATALOG-000007
sync: test/SHARED-CATALOG-000007
sync: test/SHARED-CATALOG-000007
sync: test/SHARED-CATALOG-000007
close: test/SHARED-CATALOG-000007
create: test/SHARED-CATALOG-000008
sync: test/SHARED-CATALOG-000008
create: test/marker.shared-catalog.000008.SHARED-CATALOG-000008
close: test/marker.shared-catalog.000008.SHARED-CATALOG-000008
remove: test/marker.shared-catalog.000007.SHARED-CATALOG-000007
sync: test
remove: test/SHARED-CATALOG-000007
sync: test/SHARED-CATALOG-000008
sync: test/SHARED-CATALOG-000008
sync: test/SHARED-CATALOG-000008
sync: test/SHARED-CATALOG-000007
sync: test/SHARED-CATALOG-000007

list test
----
SHARED-CATALOG-000008
marker.shared-catalog.000008.SHARED-CATALOG-000008
SHARED-CATALOG-000007
marker.shared-catalog.000007.SHARED-CATALOG-000007

0 comments on commit a9a079d

Please sign in to comment.