apache / apache/pouchdb

Local mapreduce cache not cleared for old revisions of existing views

Open
#9,006 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
17.6k
Forks
1.5k
PR merge metrics
No merged PRs in 30d

Description

### Issue
I have noted some surprising behavior around the mapreduce query cache for a local persistent view. Perhaps I am missing something, but it appears that each time a view is updated (e.g by PUTting a new revision of the design doc with a different `map`/`reduce` function for the view), a new cache entry [will be created for the new view revision](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/createView.js#L70). However, the previous revision is never cleared from the cache, even when [`viewCleanup` is triggered](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/index.js#L924). These revision caches will continue to accumulate and I cannot see how they would ever get removed _except by actually removing the view index, itself._

### Reproduce
I have modified [an existing mapreduce test](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/tests/mapreduce/test.persisted.js#L139) to illustrate this behavior. This test _passes_, but hopefully you can tell from the code/comments that the behavior seems problematic.

```js

it('Returns ok for viewCleanup after modifying view', async () => {
const db = new PouchDB(dbName);
const ddoc = {
_id: '_design/myview',
views: {
myview: {
map: function (doc) {
emit(doc.firstName);
}.toString()
}
}
};
const doc = {
_id: 'foo',
firstName: 'Foobar',
lastName: 'Bazman'
};
const info = await db.bulkDocs({docs: [ddoc, doc]});
ddoc._rev = info[0].rev;

let res = await db.query('myview');
res.rows.should.deep.equal([
{id: 'foo', key: 'Foobar', value: null}
]);

// Cache created for myview along with an entry in the meta doc
const metaDoc0 = await db.get('_local/mrviews');
Object.keys(metaDoc0.views['myview/myview']).should.have.length(1);
Object.keys(db._cachedViews).should.have.length(1);

// Update myview to have a different map function
ddoc.views.myview.map = function (doc) {
emit(doc.lastName);
}.toString();
await db.put(ddoc);
res = await db.query('myview');
res.rows.should.deep.equal([
{id: 'foo', key: 'Bazman', value: null}
]);

// New cache created for myview along with a new entry in the meta doc
const metaDoc1 = await db.get('_local/mrviews');
Object.keys(metaDoc1.views['myview/myview']).should.have.length(2);
Object.keys(db._cachedViews).should.have.length(2);

await db.viewCleanup();

// View cleanup does not remove the cache for the old version of myview
const metaDoc2 = await db.get('_local/mrviews');
metaDoc2.should.deep.equal(metaDoc1);
Object.keys(db._cachedViews).should.have.length(2);

// Add new ddoc with view that matches original version of myview
const newDdoc = {
_id: '_design/mynewview',
views: {
mynewview: {
map: function (doc) {
emit(doc.firstName);
}.toString()
}
}
};
await db.put(newDdoc);
res = await db.query('mynewview');
res.rows.should.deep.equal([
{id: 'foo', key: 'Foobar', value: null}
]);

// Meta doc not updated since mynewview is a cache collision with original myview cache
const metaDoc3 = await db.get('_local/mrviews');
metaDoc3.should.deep.equal(metaDoc2);
Object.keys(db._cachedViews).should.have.length(2);

// Remove myview from ddoc
const ddocClearViews = await db.get(ddoc._id);
ddocClearViews.views = {};
await db.put(ddocClearViews);
await db.viewCleanup();

// No updates to the meta doc
const metaDoc4 = await db.get('_local/mrviews');
metaDoc4.should.deep.equal(metaDoc3);
// Both cached views are removed (even though one was still "used" by mynewview).
Object.keys(db._cachedViews).should.have.length(0);
});
```

To break this down a little more, when a local persistent view is first queried, [a new db is created](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/createView.js#L38) to maintain the view data and a reference to this db/view [is added to the parent db's `cachedViews` array](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/createView.js#L70). Also, an entry for the _view db_ is [added to the `_local/mrviews` doc](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/createView.js#L38) for the parent db and a `destroyed` hook [is added to the view db](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/createView.js#L62) to remove the cache entry _when the view db is deleted_.

When a new revision is created for an existing view index (e.g. updated `map` function) and the view is queried, there will be a cache miss because the cache is [keyed by the actual map/reduce code for the view](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/createView.js#L12). This means that a new view db will be created and cached following the same logic as above. The revisions also accumulate in the `_local/mrviews` doc. E.g.:

```json
{
"views": {
"medic-client/contacts_by_freetext": {
"medic-user-chw-mrview-5767fa5694897717c301bcae650cf19a": true,
"medic-user-chw-mrview-08ec98c1527e69c9db02a38d33ef89b3": true,
"medic-user-chw-mrview-4644404bae3d2481974ac0b17e00cec1": true
}
}
}
```

Then, when `viewCleanup` is triggered, the contents of this metadoc drives what happens. All the design docs listed in the metadoc are [retrieved from the db](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/index.js#L945). Then, for each cached view listed in the metadoc, it checks to see [if there is still a design doc/view in the database with a matching name](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/index.js#L959). If the design doc and/or the view have been removed, the view db from the metadoc [is destroyed](https://github.com/pouchdb/pouchdb/blob/61953d170b2109855681a14e04176027340575eb/packages/node_modules/pouchdb-abstract-mapreduce/src/index.js#L972). This will trigger the `destroyed` hook to also remove the cache entry.

But, all this `viewCleanup` seems solely focused on the design/view name. Having multiple view db entries in the metadoc for a single view does not seem to be considered sufficient cause for deleting the extra view dbs. So, these revisions continue to accumulate until the view and/or design is removed.

Tangentially related (and also illustrated by the above test case) is that since `_cachedViews` is keyed on the "view signature" (from the map/reduce functions) then other views/designs that have the same view signature would hit on the same cache entry and a duplicate view db will not be build. However, the `viewCleanup` logic operates based on the `_local/mrviews` contents which are keyed by the design/view name this can result in unexpected behavior as noted in the test where a call to `viewCleanup` could end up forcing an existing view to be rebuilt.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.