Skip to content

Commit 14fc895

Browse files
committed
CBG-5355: fix panic for on demand get when ErrImportCancelled is retruned from import
1 parent 3df9aee commit 14fc895

2 files changed

Lines changed: 75 additions & 0 deletions

File tree

db/crud.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,12 @@ func (c *DatabaseCollection) GetDocSyncData(ctx context.Context, docid string) (
193193
if importErr != nil {
194194
return emptySyncData, importErr
195195
}
196+
// importDoc swallows ErrImportCancelled (e.g. SG purge race), returning
197+
// (nil, nil). Treat that the same as not found.
198+
if doc == nil {
199+
base.DebugfCtx(ctx, base.KeyImport, "Unable to import doc %q during on demand import for get - will be treated as not found.", base.UD(docid))
200+
return emptySyncData, base.ErrNotFound
201+
}
196202
}
197203

198204
return doc.SyncData, nil

db/import_test.go

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
"log"
1818
"math"
1919
"net/http"
20+
"sync"
2021
"testing"
2122
"time"
2223

@@ -1488,6 +1489,74 @@ func TestImportWithSyncCVAndNoVV(t *testing.T) {
14881489

14891490
}
14901491

1492+
// TestGetDocSyncDataPanicOnImportCancelled reproduces a race condition where importDoc
1493+
// returns (nil, nil) via ErrImportCancelled when fetching sync data through GetDocSyncData
1494+
//
1495+
// Race setup:
1496+
// 1. An SDK Delete tombstones a doc triggering on demand import for get pathway so OnDemandImportForGet is called with isDelete=true.
1497+
// 2. The first WriteUpdateWithXattrs callback succeeds and produces an updatedDoc for the
1498+
// import tombstone write.
1499+
// 3. LeakyDataStore.UpdateCallback fires SetRaw resurrects the tombstone as a live document with
1500+
// no _sync xattr, advancing the CAS.
1501+
// 4. CAS mismatch detected and retries. On retry it reads the now-live doc
1502+
// (body != nil, no _sync). The CAS mismatch block in the import callback re-fetches
1503+
// the body. Execution falls through to isDelete && doc.GetRevTreeID() == "", which fires and returns ErrImportCancelled.
1504+
// 5. importDoc's switch has no return statement in the ErrImportCancelled case, so it
1505+
// falls through to return docOut, nil with docOut==nil.
1506+
func TestGetDocSyncDataPanicOnImportCancelled(t *testing.T) {
1507+
base.SkipImportTestsIfNotEnabled(t)
1508+
base.SetUpTestLogging(t, base.LevelDebug, base.KeyCRUD, base.KeyImport)
1509+
1510+
docID := t.Name()
1511+
1512+
db, ctx := setupTestLeakyDBWithCacheOptions(t, DefaultCacheOptions(), base.LeakyBucketConfig{})
1513+
defer db.Close(ctx)
1514+
1515+
collection, ctx := GetSingleDatabaseCollectionWithUser(ctx, t, db)
1516+
docDatastore := collection.GetCollectionDatastore()
1517+
1518+
leakyDataStore, ok := base.AsLeakyDataStore(docDatastore)
1519+
require.True(t, ok)
1520+
1521+
// resurrectOnce ensures the SetRaw resurrection only fires on the first
1522+
// WriteUpdateWithXattrs attempt for the import, not on the CAS-mismatch retry,
1523+
// preventing an infinite CAS loop.
1524+
var resurrectOnce sync.Once
1525+
// importTriggered gates the callback so the SetRaw resurrection only fires
1526+
// during the import's WriteUpdateWithXattrs call, not during the initial Put.
1527+
var importTriggered bool
1528+
leakyDataStore.SetUpdateCallback(func(key string) {
1529+
// UpdateCallback fires AFTER the import callback returns but BEFORE import
1530+
// commits the tombstone write. Calling SetRaw here resurrects the tombstone as a
1531+
// live document with no _sync xattr, advancing the CAS. SGW detects the mismatch and retries. On retry the import
1532+
// callback sees body != nil and _sync RevTreeID == "", satisfying the
1533+
// isDelete && GetRevTreeID() == "" condition that returns ErrImportCancelled.
1534+
if key != docID || !importTriggered {
1535+
return
1536+
}
1537+
resurrectOnce.Do(func() {
1538+
_ = docDatastore.SetRaw(key, 0, nil, []byte(`{"foo":"resurrected"}`))
1539+
})
1540+
})
1541+
1542+
// Create doc via SG to establish a _sync xattr with a RevTreeID and a recorded CAS.
1543+
_, _, err := collection.Put(ctx, docID, Body{"foo": "bar"})
1544+
require.NoError(t, err)
1545+
db.WaitForPendingChanges(t)
1546+
1547+
// SDK-style Delete triggering isSgWrite=false inside GetDocSyncData
1548+
// which will trigger on-demand import with isDelete=true (rawDoc==nil).
1549+
err = docDatastore.Delete(docID)
1550+
require.NoError(t, err)
1551+
1552+
// UpdateCallback now acts only during the import write, not Put.
1553+
importTriggered = true
1554+
1555+
// Ensure GetDocSyncData will handling nil doc returned from on demand import event when ErrImportCancelled returned
1556+
_, err = collection.GetDocSyncData(ctx, docID)
1557+
require.Error(t, err, "expected an error when import is cancelled mid-flight, not a panic")
1558+
}
1559+
14911560
// getBucketDocument reads the current version of a document and turns it into a sgbucket.BucketDocument. This is
14921561
// intended for test use only, since this gets expiry as a separate option.
14931562
func getBucketDocument(t *testing.T, collection *DatabaseCollection, docID string) *sgbucket.BucketDocument {

0 commit comments

Comments
 (0)