CBG-4542: do not add to rev cache on write unless configured to do so

db/crud.go

@@ -2909,11 +2909,14 @@ func (db *DatabaseCollectionWithUser) updateAndReturnDoc(ctx context.Context, do
 			CV:          &Version{SourceID: doc.HLV.SourceID, Value: doc.HLV.Version},
 		}
 
-		if updateRevCache {
-			if createNewRevIDSkipped {
-				db.revisionCache.Upsert(ctx, documentRevision)
-			} else {
-				db.revisionCache.Put(ctx, documentRevision)
+		// only insert to revision cache on write if configured to do so
+		if db.dbCtx.Options.RevisionCacheOptions != nil && db.dbCtx.Options.RevisionCacheOptions.InsertOnWrite {
+			if updateRevCache {
+				if createNewRevIDSkipped {
+					db.revisionCache.Upsert(ctx, documentRevision)
+				} else {
+					db.revisionCache.Put(ctx, documentRevision)
+				}
 			}
 		}
 

db/revision_cache_interface.go

@@ -111,9 +111,10 @@ func NewRevisionCache(cacheOptions *RevisionCacheOptions, backingStores map[uint
 }
 
 type RevisionCacheOptions struct {
-	MaxItemCount uint32
-	MaxBytes     int64
-	ShardCount   uint16
+	MaxItemCount  uint32
+	MaxBytes      int64
+	ShardCount    uint16
+	InsertOnWrite bool
 }
 
 func DefaultRevisionCacheOptions() *RevisionCacheOptions {

db/revision_cache_test.go

@@ -297,8 +297,9 @@ func TestLRURevisionCacheEvictionMemoryBased(t *testing.T) {
 		t.Run(testCase.name, func(t *testing.T) {
 			dbcOptions := DatabaseContextOptions{
 				RevisionCacheOptions: &RevisionCacheOptions{
-					MaxBytes:     725,
-					MaxItemCount: 10,
+					MaxBytes:      725,
+					MaxItemCount:  10,
+					InsertOnWrite: true, // for ease of testing, have insert on write enabled
 				},
 			}
 			db, ctx := SetupTestDBWithOptions(t, dbcOptions)
@@ -711,6 +712,9 @@ func TestPutRevisionCacheAttachmentProperty(t *testing.T) {
 	rev1id, _, err := collection.Put(ctx, rev1key, rev1body)
 	assert.NoError(t, err, "Unexpected error calling collection.Put")
 
+	_, err = collection.getRev(ctx, rev1key, rev1id, 0, nil) // preload rev cache
+	require.NoError(t, err)
+
 	// Get the raw document directly from the bucket, validate _attachments property isn't found
 	var bucketBody Body
 	_, err = collection.dataStore.Get(rev1key, &bucketBody)
@@ -1364,29 +1368,29 @@ func TestRevisionCacheRemove(t *testing.T) {
 	docRev, err := collection.revisionCache.GetWithRev(base.TestCtx(t), "doc", rev1id, true)
 	assert.NoError(t, err)
 	assert.Equal(t, rev1id, docRev.RevID)
-	assert.Equal(t, int64(0), db.DbStats.Cache().RevisionCacheMisses.Value())
+	assert.Equal(t, int64(1), db.DbStats.Cache().RevisionCacheMisses.Value())
 
 	collection.revisionCache.RemoveWithRev(ctx, "doc", rev1id)
 
 	docRev, err = collection.revisionCache.GetWithRev(base.TestCtx(t), "doc", rev1id, true)
 	assert.NoError(t, err)
 	assert.Equal(t, rev1id, docRev.RevID)
-	assert.Equal(t, int64(1), db.DbStats.Cache().RevisionCacheMisses.Value())
+	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheMisses.Value())
 
 	docRev, err = collection.revisionCache.GetActive(ctx, "doc")
 	assert.NoError(t, err)
 	assert.Equal(t, rev1id, docRev.RevID)
-	assert.Equal(t, int64(1), db.DbStats.Cache().RevisionCacheMisses.Value())
+	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheMisses.Value())
 
 	docRev, err = collection.GetRev(ctx, "doc", docRev.RevID, true, nil)
 	assert.NoError(t, err)
 	assert.Equal(t, rev1id, docRev.RevID)
-	assert.Equal(t, int64(1), db.DbStats.Cache().RevisionCacheMisses.Value())
+	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheMisses.Value())
 
 	docRev, err = collection.GetRev(ctx, "doc", "", true, nil)
 	assert.NoError(t, err)
 	assert.Equal(t, rev1id, docRev.RevID)
-	assert.Equal(t, int64(1), db.DbStats.Cache().RevisionCacheMisses.Value())
+	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheMisses.Value())
 }
 
 // TestRevCacheHitMultiCollection:
@@ -1437,8 +1441,21 @@ func TestRevCacheHitMultiCollection(t *testing.T) {
 	}
 
 	// assert that both docs were found in rev cache and no cache misses are being reported
+	assert.Equal(t, int64(0), db.DbStats.Cache().RevisionCacheHits.Value())
+	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheMisses.Value())
+
+	// Perform a get for the doc in each collection again asserting we get from rev cache this time
+	for i, collection := range collectionList {
+		ctx := collection.AddCollectionContext(ctx)
+		docRev, err := collection.GetRev(ctx, "doc", revList[i], false, nil)
+		require.NoError(t, err)
+		assert.Equal(t, "doc", docRev.DocID)
+		assert.Equal(t, revList[i], docRev.RevID)
+	}
+
+	// assert two hits now recorded
 	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheHits.Value())
-	assert.Equal(t, int64(0), db.DbStats.Cache().RevisionCacheMisses.Value())
+	assert.Equal(t, int64(2), db.DbStats.Cache().RevisionCacheMisses.Value())
 }
 
 // TestRevCacheHitMultiCollectionLoadFromBucket:
@@ -1839,6 +1856,10 @@ func createDocAndReturnSizeAndRev(t *testing.T, ctx context.Context, docID strin
 		expectedSize += len([]byte(v))
 	}
 
+	// do fetch ro load into cache
+	_, err = collection.getRev(ctx, docID, rev, 0, nil)
+	require.NoError(t, err)
+
 	return expectedSize, rev, doc.HLV.ExtractCurrentVersionFromHLV()
 }
 
@@ -2263,18 +2284,19 @@ func TestRevCacheOnDemandImportNoCache(t *testing.T) {
 	revID1, _, err := collection.Put(ctx, docID, Body{"foo": "bar"})
 	require.NoError(t, err)
 
+	// rev 1 is not in cache given we don;t write to cache on write
 	_, exists := collection.revisionCache.Peek(ctx, docID, revID1)
-	require.True(t, exists)
+	require.False(t, exists)
 
 	require.NoError(t, collection.dataStore.Set(docID, 0, nil, []byte(`{"foo": "baz"}`)))
 
 	doc, err := collection.GetDocument(ctx, docID, DocUnmarshalSync)
 	require.NoError(t, err)
 	require.Equal(t, Body{"foo": "baz"}, doc.Body(ctx))
 
-	// rev1 still exists in cache but not on server
+	// rev1 still won;t exist in cache
 	_, exists = collection.revisionCache.Peek(ctx, docID, revID1)
-	require.True(t, exists)
+	require.False(t, exists)
 
 	// rev2 is not in cache but is on server
 	_, exists = collection.revisionCache.Peek(ctx, docID, doc.GetRevTreeID())

docs/api/components/schemas.yaml

@@ -1105,6 +1105,12 @@ Database:
                 you may find that the capacity stat (revision_cache_num_items) will climb to the defined rev cache size + 10%.
               type: integer
               default: 5000
+            insert_on_write:
+              description: |-
+                Whether to insert revisions into the revision cache when documents are written to the database.
+                If false, only revisions read from the database will be cached.
+              type: boolean
+              default: false
         channel_cache_expiry:
           description: |-
             **Deprecated, please use the database setting `cache.channel_cache.expiry_seconds` instead**

rest/attachment_test.go

@@ -2991,6 +2991,11 @@ func TestGetNonWinningRevisionAttachmentLeak(t *testing.T) {
 					RequireStatus(t, resp, http.StatusCreated)
 					version1 := DocVersionFromPutResponse(t, resp)
 
+					if !flushCache {
+						// ensure revision is cached
+						_, _ = rt.GetDoc(docID)
+					}
+
 					// version 2 moves to channel B (alice has no access) with an attachment
 					resp = rt.SendAdminRequest(http.MethodPut, "/{{.keyspace}}/"+docID+"?rev="+version1.RevTreeID,
 						`{"channels": ["NBC"], "value": "v2", "_attachments": {"attachment.txt": {"data": "YXR0ZGF0YQ=="}}}`)

rest/changestest/changes_api_test.go

@@ -1756,8 +1756,7 @@ func updateTestDoc(rt *rest.RestTester, docid string, revid string, body string)
 
 // Validate retrieval of various document body types using include_docs.
 func TestChangesIncludeDocs(t *testing.T) {
-	base.SetUpTestLogging(t, base.LevelInfo, base.KeyNone)
-	t.Skip("pending CBG-4542")
+	base.SetUpTestLogging(t, base.LevelInfo, base.KeyAll)
 
 	rtConfig := rest.RestTesterConfig{SyncFn: `function(doc) {channel(doc.channels)}`}
 	rt := rest.NewRestTester(t, &rtConfig)
@@ -1859,7 +1858,8 @@ func TestChangesIncludeDocs(t *testing.T) {
 	expectedResults[2] = `{"seq":4,"id":"doc_multi_rev","doc":{"_id":"doc_multi_rev","_rev":"2-db2cf770921c3764b2d213ee0cbb5f45","channels":["alpha"],"type":"active","v":2},"changes":[{"rev":"2-db2cf770921c3764b2d213ee0cbb5f45"}]}`
 	expectedResults[3] = `{"seq":6,"id":"doc_tombstone","deleted":true,"removed":["alpha"],"doc":{"_deleted":true,"_id":"doc_tombstone","_rev":"2-5bd8eb422f30e8d455940672e9e76549"},"changes":[{"rev":"2-5bd8eb422f30e8d455940672e9e76549"}]}`
 	expectedResults[4] = `{"seq":8,"id":"doc_removed","removed":["alpha"],"doc":{"_id":"doc_removed","_removed":true,"_rev":"2-d15cb77d1dbe1cc06d27310de5b75914"},"changes":[{"rev":"2-d15cb77d1dbe1cc06d27310de5b75914"}]}`
-	expectedResults[5] = `{"seq":12,"id":"doc_pruned","removed":["alpha"],"doc":{"_id":"doc_pruned","_removed":true,"_rev":"2-5afcb73bd3eb50615470e3ba54b80f00"},"changes":[{"rev":"2-5afcb73bd3eb50615470e3ba54b80f00"}]}`
+	// for doc_pruned 4.0 cannot differentiate between channel removal or missing document - document body will not be returned here
+	expectedResults[5] = `{"seq":12,"id":"doc_pruned","removed":["alpha"],"changes":[{"rev":"2-5afcb73bd3eb50615470e3ba54b80f00"}]}`
 	expectedResults[6] = `{"seq":18,"id":"doc_attachment","doc":{"_attachments":{"attach1":{"content_type":"text/plain","digest":"sha1-nq0xWBV2IEkkpY3ng+PEtFnCcVY=","length":30,"revpos":2,"stub":true}},"_id":"doc_attachment","_rev":"2-0b0457923508d99ec1929d2316d14cf2","channels":["alpha"],"type":"attachments"},"changes":[{"rev":"2-0b0457923508d99ec1929d2316d14cf2"}]}`
 	expectedResults[7] = `{"seq":19,"id":"doc_large_numbers","doc":{"_id":"doc_large_numbers","_rev":"1-2721633d9000e606e9c642e98f2f5ae7","channels":["alpha"],"largefloat":1234567890.1234,"largeint":1234567890,"type":"large_numbers"},"changes":[{"rev":"1-2721633d9000e606e9c642e98f2f5ae7"}]}`
 	expectedResults[8] = `{"seq":22,"id":"doc_conflict","doc":{"_id":"doc_conflict","_rev":"2-conflicting_rev","channels":["alpha"],"type":"conflict"},"changes":[{"rev":"2-conflicting_rev"}]}`

rest/config.go

@@ -252,6 +252,7 @@ type RevCacheConfig struct {
 	MaxItemCount     *uint32 `json:"size,omitempty"`                // Maximum number of revisions to store in the revision cache
 	MaxMemoryCountMB *uint32 `json:"max_memory_count_mb,omitempty"` // Maximum amount of memory the rev cache should consume in MB, when configured it will work in tandem with max items
 	ShardCount       *uint16 `json:"shard_count,omitempty"`         // Number of shards the rev cache should be split into
+	InsertOnWrite    *bool   `json:"insert_on_write,omitempty"`     // Whether to insert revisions into the cache on document writes
 }
 
 type ChannelCacheConfig struct {

rest/revocation_test.go

@@ -2452,6 +2452,18 @@ func TestRevocationGetSyncDataError(t *testing.T) {
 						return nil
 					},
 				},
+				// we need to insert on write to rev cache for this test as the LeakyBucketConfig above
+				// intercepts the GetWithXattr call when attempting to load revision from bucket for sending revision
+				// to client
+				DatabaseConfig: &DatabaseConfig{
+					DbConfig: DbConfig{
+						CacheConfig: &CacheConfig{
+							RevCacheConfig: &RevCacheConfig{
+								InsertOnWrite: base.Ptr(true),
+							},
+						},
+					},
+				},
 			},
 		)
 

rest/server_context.go

@@ -1232,6 +1232,9 @@ func dbcOptionsFromConfig(ctx context.Context, sc *ServerContext, config *DbConf
 			if config.CacheConfig.RevCacheConfig.ShardCount != nil {
 				revCacheOptions.ShardCount = *config.CacheConfig.RevCacheConfig.ShardCount
 			}
+			if config.CacheConfig.RevCacheConfig.InsertOnWrite != nil {
+				revCacheOptions.InsertOnWrite = *config.CacheConfig.RevCacheConfig.InsertOnWrite
+			}
 		}
 	}