From c9d89ba0c41c9e927b9e543b042e1fe4f93489ff Mon Sep 17 00:00:00 2001
From: Richard Petersen <richard.petersen@open-xchange.com>
Date: Tue, 24 May 2022 16:26:29 +0200
Subject: [PATCH] fixed: OXUIB-1631 UI-middleware memory spikes after startup
(3)
Cache lookups will immediately register a promise to prevent multiple requests for the same ressource
---
spec/file_caching_test.js | 36 +++++++++++++++++++++
src/cache.js | 47 +++++++++++++++------------
src/files.js | 59 ++++++++++++++++++++++++----------
src/manifests.js | 36 +++++++--------------
src/meta.js | 8 ++---
src/middlewares/serve-files.js | 11 ++-----
6 files changed, 121 insertions(+), 76 deletions(-)
diff --git a/spec/file_caching_test.js b/spec/file_caching_test.js
index db08ac9..6c6c032 100644
--- a/spec/file_caching_test.js
+++ b/spec/file_caching_test.js
@@ -312,4 +312,40 @@ describe('File caching service', function () {
expect(res2.statusCode).to.equal(200)
expect(spy.callCount).to.equal(1)
})
+
+ describe('redis request latency', function () {
+ beforeEach(function () {
+ // overwrite redis.get to simulate network latency
+ const get = redis.client.get
+ redis.client.get = function () {
+ return new Promise(resolve => {
+ setTimeout(async () => {
+ resolve(await get.apply(redis.client, arguments))
+ }, 20)
+ })
+ }
+ })
+
+ it('only requests files once, even though the response takes some time', async function () {
+ let spy
+ mockFetch({
+ 'http://ui-server': {
+ '/manifest.json': generateSimpleViteManifest({}),
+ '/example.js': spy = sandbox.spy(() => {
+ return new Response('this is example', { headers: { 'content-type': 'application/javascript' } })
+ })
+ }
+ })
+ app = await mockApp()
+
+ expect(spy.callCount).to.equal(0)
+ const [res1, res2] = await Promise.all([
+ request(app).get('/example.js'),
+ request(app).get('/example.js')
+ ])
+ expect(res1.statusCode).to.equal(200)
+ expect(res2.statusCode).to.equal(200)
+ expect(spy.callCount).to.equal(1)
+ })
+ })
})
diff --git a/src/cache.js b/src/cache.js
index 1d64904..505849c 100644
--- a/src/cache.js
+++ b/src/cache.js
@@ -3,14 +3,6 @@ import * as redis from './redis.js'
const cache = {}
-export async function setAsync (key, asyncValue) {
- logger.trace(`[Cache] Set async ${key}`)
- cache[key] = asyncValue
- const value = await asyncValue
- await set(key, value)
- return value
-}
-
export function set (key, value) {
logger.trace(`[Cache] Set ${key}`)
if (cache[key] === value) return
@@ -20,28 +12,43 @@ export function set (key, value) {
}
}
-export async function getBuffer (key) {
- return get(key, { method: 'getBuffer' })
-}
-
export async function clear () {
for (const prop of Object.getOwnPropertyNames(cache)) {
delete cache[prop]
}
}
-export async function get (key, { method = 'get' } = {}) {
+export function get (key, fallback) {
if (cache[key]) {
logger.trace(`[Cache] Resolve "${key}" from memory`)
return cache[key]
}
- if (redis.isEnabled()) {
- const result = await redis.client[method]?.(key)
- if (result) {
- logger.trace(`[Cache] Resolve "${key}" from redis`)
- cache[key] = result
+ const promise = (async () => {
+ if (redis.isEnabled()) {
+ let result = await redis.client.get(key)
+ if (result) {
+ logger.trace(`[Cache] Resolve "${key}" from redis`)
+ result = JSON.parse(result)
+ cache[key] = result
+ return result
+ }
}
- return result
- }
+
+ if (!fallback) return
+
+ const fallbackResult = await fallback()
+ if (fallbackResult) {
+ logger.trace(`[Cache] Found a fallback for "${key}"`)
+ cache[key] = fallbackResult
+ if (redis.isEnabled()) redis.client.set(key, JSON.stringify(fallbackResult))
+ }
+ return fallbackResult
+ })()
+
+ cache[key] = promise
+
+ return promise
}
+
+export function getCache () { return cache }
diff --git a/src/files.js b/src/files.js
index 6f1261e..ff0f20f 100644
--- a/src/files.js
+++ b/src/files.js
@@ -6,6 +6,12 @@ import { getCSSDependenciesFor, getViteManifests } from './manifests.js'
import * as cache from './cache.js'
import { logger } from './logger.js'
import { NotFoundError } from './errors.js'
+import * as redis from './redis.js'
+
+export function createWritable (body) {
+ if (typeof body !== 'string' && !(body instanceof Buffer)) return JSON.stringify(body)
+ return body
+}
export async function fetchFileWithHeadersFromBaseUrl (path, baseUrl, version) {
const [response, dependencies] = await Promise.all([
@@ -50,22 +56,41 @@ export async function fetchFileWithHeaders ({ path, version }) {
return Promise.any(config.urls.map(baseUrl => fetchFileWithHeadersFromBaseUrl(path, baseUrl, version)))
}
-export async function saveAsyncToCache ({ version, path, promise }) {
- await Promise.all([
- cache.setAsync(getRedisKey({ version, name: `${path}:body` }), promise.then(({ body }) => {
- if (typeof body !== 'string' && !(body instanceof Buffer)) return JSON.stringify(body)
- return body
- })),
- cache.setAsync(getRedisKey({ version, name: `${path}:meta` }), promise.then(({ body, ...rest }) => JSON.stringify(rest)))
- ])
- return promise
-}
+export function getFile ({ version, path }) {
+ const key = getRedisKey({ version, name: `${path}` })
-export async function loadFromCache ({ version, path }) {
- const [body, meta = '{}'] = await Promise.all([
- cache.getBuffer(getRedisKey({ version, name: `${path}:body` })),
- cache.get(getRedisKey({ version, name: `${path}:meta` }))
- ])
- if (!body) return
- return { ...JSON.parse(meta), body }
+ // try to get the file synchronously.
+ const data = cache.getCache()[key]
+ if (data) return data
+
+ // if synchronously does not work, store the async promise for further requests
+ const promise = (async () => {
+ const bodyKey = getRedisKey({ version, name: `${path}:body` })
+ const metaKey = getRedisKey({ version, name: `${path}:meta` })
+
+ if (redis.isEnabled()) {
+ const [body, meta = '{}'] = await Promise.all([
+ redis.client.getBuffer(bodyKey),
+ redis.client.get(metaKey)
+ ])
+
+ if (body) {
+ return (cache.getCache()[key] = { body, ...JSON.parse(meta) })
+ }
+ }
+
+ const dataFromServer = await fetchFileWithHeaders({ version, path })
+ if (redis.isEnabled()) {
+ const { body, ...rest } = dataFromServer
+ redis.client.set(bodyKey, createWritable(body))
+ redis.client.set(metaKey, JSON.stringify(rest))
+ }
+
+ // overwrite cache with synchronous data
+ return (cache.getCache()[key] = dataFromServer)
+ })()
+
+ // temporary set to promise
+ cache.getCache()[key] = promise
+ return promise
}
diff --git a/src/manifests.js b/src/manifests.js
index 94bd2dd..7f6ed5d 100644
--- a/src/manifests.js
+++ b/src/manifests.js
@@ -28,34 +28,22 @@ export async function fetchViteManifests () {
return viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
}
-export async function getViteManifests ({ version }) {
- let manifests = await cache.get(getRedisKey({ version, name: 'viteManifests' }))
- if (!manifests) {
- manifests = await cache.setAsync(getRedisKey({ version, name: 'viteManifests' }), fetchViteManifests().then(m => JSON.stringify(m)))
- }
- return JSON.parse(manifests)
+export function getViteManifests ({ version }) {
+ return cache.get(getRedisKey({ version, name: 'viteManifests' }), () => fetchViteManifests())
}
-export async function getOxManifests ({ version }) {
- let manifests = await cache.get(getRedisKey({ version, name: 'oxManifests' }))
- if (!manifests) {
- manifests = await cache.setAsync(getRedisKey({ version, name: 'oxManifests' }), (async () => {
- const viteManifests = await getViteManifests({ version })
- return JSON.stringify(viteToOxManifest(viteManifests))
- })())
- }
- return JSON.parse(manifests)
+export function getOxManifests ({ version }) {
+ return cache.get(getRedisKey({ version, name: 'oxManifests' }), async () => {
+ const viteManifests = await getViteManifests({ version })
+ return viteToOxManifest(viteManifests)
+ })
}
-export async function getDependencies ({ version }) {
- let deps = await cache.get(getRedisKey({ version, name: 'dependencies' }))
- if (!deps) {
- deps = await await cache.setAsync(getRedisKey({ version, name: 'dependencies' }), (async () => {
- const viteManifests = await getViteManifests({ version })
- return JSON.stringify(viteManifestToDeps(viteManifests))
- })())
- }
- return JSON.parse(deps)
+export function getDependencies ({ version }) {
+ return cache.get(getRedisKey({ version, name: 'dependencies' }), async () => {
+ const viteManifests = await getViteManifests({ version })
+ return viteManifestToDeps(viteManifests)
+ })
}
export async function getCSSDependenciesFor ({ file, version }) {
diff --git a/src/meta.js b/src/meta.js
index 0b99feb..386c78b 100644
--- a/src/meta.js
+++ b/src/meta.js
@@ -27,10 +27,6 @@ export async function fetchMergedMetadata () {
return metadata.filter(Boolean)
}
-export async function getMergedMetadata ({ version }) {
- let metadata = await cache.get(getRedisKey({ version, name: 'mergedMetadata' }))
- if (!metadata) {
- metadata = await cache.setAsync(getRedisKey({ version, name: 'mergedMetadata' }), fetchMergedMetadata().then(m => JSON.stringify(m)))
- }
- return JSON.parse(metadata)
+export function getMergedMetadata ({ version }) {
+ return cache.get(getRedisKey({ version, name: 'mergedMetadata' }), () => fetchMergedMetadata())
}
diff --git a/src/middlewares/serve-files.js b/src/middlewares/serve-files.js
index 9f17245..1ef2cb4 100644
--- a/src/middlewares/serve-files.js
+++ b/src/middlewares/serve-files.js
@@ -1,4 +1,4 @@
-import { fetchFileWithHeaders, loadFromCache, saveAsyncToCache } from '../files.js'
+import { getFile } from '../files.js'
import { NotFoundError } from '../errors.js'
import createError from 'http-errors'
@@ -7,15 +7,8 @@ export default async function (req, res, next) {
if (req.method !== 'GET') return next()
const version = res.version
const path = req.path === '/' ? '/index.html' : req.path
- let data = await loadFromCache({
- path,
- version
- })
- if (!data) {
- data = await saveAsyncToCache({ version, path, promise: fetchFileWithHeaders({ path, version }) })
- }
+ const { body, headers, sha256Sum } = await getFile({ version, path })
- const { body, headers, sha256Sum } = data
res.set(headers)
res.locals.sha256Sum = sha256Sum
res.send(body)
--
GitLab