Skip to content

Commit e3928ff

Browse files
committed
Fix QueueCache bugs and enable engine caching
QueueCache was disabled in 2015, so every request was rebuilding engines from scratch. This fixes bugs and enables it. - Replace Promise.delay() (Bluebird, no longer installed) with native setTimeout - Set working flag on resolved engine instead of on the Promise object, which left the mutex broken and theoretically allowed concurrent requests for the same style to corrupt each other's results - Add 5s timeout on busy-wait retry loop, replacing the stuck cache entry with a fresh engine so subsequent requests aren't penalized - Fix inverted LRU eviction sort (was evicting hottest items) - Set engineCacheSize to 20
1 parent 9803253 commit e3928ff

File tree

3 files changed

+22
-10
lines changed

3 files changed

+22
-10
lines changed

config/default.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"cslPath" : "./csl",
66
"renamedStylesPath": "./csl/renamed-styles.json",
77
"cslDependentPath": "./csl/dependent",
8-
"engineCacheSize" : 40,
8+
"engineCacheSize" : 20,
99
"timings": false,
1010
"debug": true,
1111
"userAgent": "Zotero Citation Server (https://github.com/zotero/citeproc-js-server)"

lib/citeServer.js

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,7 @@ var requestCount = 0;
116116
// Instantiate resource managers
117117
let cslLoader = new styles.CslLoader(config);
118118
let localeManager = new locales.LocaleManager(config.localesPath);
119-
//let engineCache = new enginecaching.QueueCache(config);
120-
let engineCache = new enginecaching.NoncacheEngineCache(config);
119+
let engineCache = new enginecaching.QueueCache(config);
121120
engineCache.localeManager = localeManager;
122121
engineCache.cslLoader = cslLoader;
123122

lib/engineCaching.js

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,12 @@ let citeproc = require('./citeprocnode');
3434
let path = require('path');
3535
let jsonWalker = require("./json_walker.js");
3636

37+
function delay(ms) {
38+
return new Promise(function(resolve) {
39+
setTimeout(resolve, ms);
40+
});
41+
}
42+
3743
exports.NoncacheEngineCache = function(config){
3844
if(config){
3945
this.config = config;
@@ -106,7 +112,7 @@ exports.QueueCache.prototype.config = {
106112
* @param {[type]} locale [description]
107113
* @return {[type]} [description]
108114
*/
109-
exports.QueueCache.prototype.getEngine = function(styleUrlObj, locale) {
115+
exports.QueueCache.prototype.getEngine = function(styleUrlObj, locale, _retryStart) {
110116
let engineCache = this;
111117
log.info("engine requested with locale:", locale);
112118
//try to get a cached engine
@@ -121,15 +127,21 @@ exports.QueueCache.prototype.getEngine = function(styleUrlObj, locale) {
121127
log.info("No cached engine found");
122128
let newEngine = engineCache.buildNewEngine(styleUrlObj, locale);
123129
engineCache.cachedEngines[cacheEngineString] = newEngine;
124-
newEngine.working = true;
125130
return newEngine;
126131
} else {
127132
log.info("cached engine found");
128133
return engineCache.cachedEngines[cacheEngineString].then(function(citeprocEngine){
129134
if(citeprocEngine instanceof citeproc.CiteprocEngine){
130135
if(citeprocEngine.working){
131-
return Promise.delay(10).then(function(){
132-
return engineCache.getEngine(styleUrlObj, locale);
136+
let retryStart = _retryStart || Date.now();
137+
if(Date.now() - retryStart > 5000){
138+
log.warn("Timed out waiting for engine -- replacing stuck cache entry:", cacheEngineString);
139+
let newEngine = engineCache.buildNewEngine(styleUrlObj, locale);
140+
engineCache.cachedEngines[cacheEngineString] = newEngine;
141+
return newEngine;
142+
}
143+
return delay(10).then(function(){
144+
return engineCache.getEngine(styleUrlObj, locale, retryStart);
133145
});
134146
}
135147
log.info("returning existing citeproc instance from QueueCache.getEngine");
@@ -187,8 +199,9 @@ exports.QueueCache.prototype.buildNewEngine = function(styleUrlObj, locale){
187199
cslObject = jsonWalker.JsonWalker.walkStyleToObj(cslDoc).obj;
188200
cslDoc.defaultView.close();
189201
}
190-
202+
191203
let citeprocEngine = new citeproc.CiteprocEngine({}, cslObject, locale, engineCache.localeManager, null);
204+
citeprocEngine.working = true;
192205
return citeprocEngine;
193206
});
194207
};
@@ -222,9 +235,9 @@ exports.QueueCache.prototype.clean = function(){
222235
gcCacheArray.push(i);
223236
}
224237

225-
//sort by last used
238+
//sort by last used (oldest first so we evict the coldest)
226239
gcCacheArray.sort(function(a, b){
227-
return engineCache.lastUsed[b] - engineCache.lastUsed[a];
240+
return (engineCache.lastUsed[a] || 0) - (engineCache.lastUsed[b] || 0);
228241
});
229242

230243
//evict a third of the cache

0 commit comments

Comments
 (0)