Skip to content
Snippets Groups Projects
Commit 43b4dd6a authored by richard.petersen's avatar richard.petersen :sailboat:
Browse files

fixed: OXUIB-1631 memory spikes immediatly after startup 

Root cause: If multiple connections are opened at the same time after an update, the ui-middleware will do multiple requests to get initial data. That went especially worse, when e2e-tests are launched against ui-middlewares with multiple clients connecting at the same time.
Soltion: Make sure, that a node already caches stuff without a response.
parent 451c9023
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
Showing
with 131 additions and 90 deletions
integration/caching_test.js
+ 1
− 1
View file @ 43b4dd6a
...@@ -41,7 +41,7 @@ describe('File caching service', function () { ...@@ -41,7 +41,7 @@ describe('File caching service', function () {
expect(response.statusCode).to.equal(200) expect(response.statusCode).to.equal(200)
const version = response.headers.version const version = response.headers.version
expect(await client.get(getRedisKey({ version, name: '/index.html:meta' }))).to.equal('{"headers":{"content-type":"text/html","dependencies":false},"sha256Sum":"iFSuC3aK6EN/ASUamuZ+j3xZXI9eBdIlxtVDFjn7y1I="}') expect(await client.get(getRedisKey({ version, name: '/index.html:meta' }))).to.equal('{"sha256Sum":"iFSuC3aK6EN/ASUamuZ+j3xZXI9eBdIlxtVDFjn7y1I=","headers":{"content-type":"text/html","dependencies":false}}')
expect(await client.get(getRedisKey({ version, name: '/index.html:body' }))).to.equal('<html><head></head><body>it\'s me</body></html>') expect(await client.get(getRedisKey({ version, name: '/index.html:body' }))).to.equal('<html><head></head><body>it\'s me</body></html>')
}) })
......
spec/file_caching_test.js
+ 45
− 0
View file @ 43b4dd6a
...@@ -267,4 +267,49 @@ describe('File caching service', function () { ...@@ -267,4 +267,49 @@ describe('File caching service', function () {
expect(response5.statusCode).to.equal(200) expect(response5.statusCode).to.equal(200)
expect(response5.text).to.equal('second') expect(response5.text).to.equal('second')
}) })
it('only fetches files once, even when requested simultanously', 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)
})
it('only fetches manifests once, even when requested simultanously', async function () {
let spy
mockFetch({
'http://ui-server': {
'/manifest.json': spy = sandbox.spy(async () => {
await new Promise(resolve => setTimeout(resolve, 10))
return new Response(JSON.stringify(generateSimpleViteManifest({})), { headers: { 'content-type': 'application/json' } })
}),
'/example.js': () => 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('/manifests'),
request(app).get('/example.js')
])
expect(res1.statusCode).to.equal(200)
expect(res2.statusCode).to.equal(200)
expect(spy.callCount).to.equal(1)
})
}) })
src/cache.js
+ 7
− 0
View file @ 43b4dd6a
...@@ -2,6 +2,13 @@ import * as redis from './redis.js' ...@@ -2,6 +2,13 @@ import * as redis from './redis.js'
const cache = {} const cache = {}
export async function setAsync (key, asyncValue) {
cache[key] = asyncValue
const value = await asyncValue
await set(key, value)
return value
}
export function set (key, value) { export function set (key, value) {
if (cache[key] === value) return if (cache[key] === value) return
cache[key] = value cache[key] = value
......
src/create-app.js
+ 2
− 6
View file @ 43b4dd6a
...@@ -7,9 +7,7 @@ import yaml from 'js-yaml' ...@@ -7,9 +7,7 @@ import yaml from 'js-yaml'
import fs from 'fs' import fs from 'fs'
import versionMiddleware from './middlewares/version.js' import versionMiddleware from './middlewares/version.js'
import loadFromCacheMiddleware from './middlewares/load-from-cache.js' import serveFilesMiddleware from './middlewares/serve-files.js'
import loadFromUIServersMiddleware from './middlewares/load-from-server.js'
import saveToCacheMiddleware from './middlewares/save-to-cache.js'
import finalHandlerMiddleware from './middlewares/final-handler.js' import finalHandlerMiddleware from './middlewares/final-handler.js'
import health from './routes/health.js' import health from './routes/health.js'
...@@ -40,14 +38,12 @@ export function createApp () { ...@@ -40,14 +38,12 @@ export function createApp () {
app.timeout = 30000 app.timeout = 30000
app.use(versionMiddleware) app.use(versionMiddleware)
app.use(loadFromCacheMiddleware)
app.use(manifestsRouter) app.use(manifestsRouter)
app.use(metadataRouter) app.use(metadataRouter)
app.use(redirectsRouter) app.use(redirectsRouter)
app.use(loadFromUIServersMiddleware) app.use(serveFilesMiddleware)
app.use(saveToCacheMiddleware)
app.use(finalHandlerMiddleware) app.use(finalHandlerMiddleware)
return app return app
......
src/files.js
+ 8
− 5
View file @ 43b4dd6a
...@@ -50,12 +50,15 @@ export async function fetchFileWithHeaders ({ path, version }) { ...@@ -50,12 +50,15 @@ export async function fetchFileWithHeaders ({ path, version }) {
return Promise.any(config.urls.map(baseUrl => fetchFileWithHeadersFromBaseUrl(path, baseUrl, version))) return Promise.any(config.urls.map(baseUrl => fetchFileWithHeadersFromBaseUrl(path, baseUrl, version)))
} }
export async function saveToCache ({ version, path, body, headers, ...rest }) { export async function saveAsyncToCache ({ version, path, promise }) {
if (typeof body !== 'string' && !(body instanceof Buffer)) body = JSON.stringify(body) await Promise.all([
return Promise.all([ cache.setAsync(getRedisKey({ version, name: `${path}:body` }), promise.then(({ body }) => {
cache.set(getRedisKey({ version, name: `${path}:body` }), body), if (typeof body !== 'string' && !(body instanceof Buffer)) return JSON.stringify(body)
cache.set(getRedisKey({ version, name: `${path}:meta` }), JSON.stringify({ headers, ...rest })) return body
})),
cache.setAsync(getRedisKey({ version, name: `${path}:meta` }), promise.then(({ body, ...rest }) => JSON.stringify(rest)))
]) ])
return promise
} }
export async function loadFromCache ({ version, path }) { export async function loadFromCache ({ version, path }) {
......
src/manifests.js
+ 26
− 21
View file @ 43b4dd6a
...@@ -4,10 +4,7 @@ import { getRedisKey, viteManifestToDeps, viteToOxManifest } from './util.js' ...@@ -4,10 +4,7 @@ import { getRedisKey, viteManifestToDeps, viteToOxManifest } from './util.js'
import { logger } from './logger.js' import { logger } from './logger.js'
import * as cache from './cache.js' import * as cache from './cache.js'
export async function getViteManifests ({ version }) { export async function fetchViteManifests () {
const manifests = await cache.get(getRedisKey({ version, name: 'viteManifests' }))
if (manifests) return JSON.parse(manifests)
// vite manifests contains a set of objects with the vite-manifests // vite manifests contains a set of objects with the vite-manifests
// from the corresponding registered services // from the corresponding registered services
const viteManifests = await Promise.all(config.urls.map(async baseUrl => { const viteManifests = await Promise.all(config.urls.map(async baseUrl => {
...@@ -28,29 +25,37 @@ export async function getViteManifests ({ version }) { ...@@ -28,29 +25,37 @@ export async function getViteManifests ({ version }) {
})) }))
// combine all manifests by keys. With duplicates, last wins // combine all manifests by keys. With duplicates, last wins
const newManifests = viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {}) return viteManifests.reduce((memo, manifest) => Object.assign(memo, manifest), {})
await cache.set(getRedisKey({ version, name: 'viteManifests' }), JSON.stringify(newManifests))
return newManifests
} }
export async function getOxManifests ({ version }) { export async function getViteManifests ({ version }) {
const manifests = await cache.get(getRedisKey({ version, name: 'oxManifests' })) let manifests = await cache.get(getRedisKey({ version, name: 'viteManifests' }))
if (manifests) return JSON.parse(manifests) if (!manifests) {
manifests = await cache.setAsync(getRedisKey({ version, name: 'viteManifests' }), fetchViteManifests().then(m => JSON.stringify(m)))
}
return JSON.parse(manifests)
}
const viteManifests = await getViteManifests({ version }) export async function getOxManifests ({ version }) {
const newManifests = viteToOxManifest(viteManifests) let manifests = await cache.get(getRedisKey({ version, name: 'oxManifests' }))
await cache.set(getRedisKey({ version, name: 'oxManifests' }), JSON.stringify(newManifests)) if (!manifests) {
return newManifests manifests = await cache.setAsync(getRedisKey({ version, name: 'oxManifests' }), (async () => {
const viteManifests = await getViteManifests({ version })
return JSON.stringify(viteToOxManifest(viteManifests))
})())
}
return JSON.parse(manifests)
} }
export async function getDependencies ({ version }) { export async function getDependencies ({ version }) {
const deps = await cache.get(getRedisKey({ version, name: 'dependencies' })) let deps = await cache.get(getRedisKey({ version, name: 'dependencies' }))
if (deps) return JSON.parse(deps) if (!deps) {
deps = await await cache.setAsync(getRedisKey({ version, name: 'dependencies' }), (async () => {
const viteManifests = await getViteManifests({ version }) const viteManifests = await getViteManifests({ version })
const newDeps = viteManifestToDeps(viteManifests) return JSON.stringify(viteManifestToDeps(viteManifests))
await cache.set(getRedisKey({ version, name: 'dependencies' }), JSON.stringify(newDeps)) })())
return newDeps }
return JSON.parse(deps)
} }
export async function getCSSDependenciesFor ({ file, version }) { export async function getCSSDependenciesFor ({ file, version }) {
......
src/meta.js
+ 12
− 9
View file @ 43b4dd6a
...@@ -3,11 +3,8 @@ import fetch from 'node-fetch' ...@@ -3,11 +3,8 @@ import fetch from 'node-fetch'
import * as cache from './cache.js' import * as cache from './cache.js'
import { getRedisKey } from './util.js' import { getRedisKey } from './util.js'
export async function getMergedMetadata ({ version }) { export async function fetchMergedMetadata () {
const metadata = await cache.get(getRedisKey({ version, name: 'mergedMetadata' })) const metadata = await Promise.all(config.urls.map(async url => {
if (metadata) return JSON.parse(metadata)
const newMetadata = await Promise.all(config.urls.map(async url => {
const { origin } = new URL(url) const { origin } = new URL(url)
try { try {
const response = await fetch(new URL('meta.json', origin)) const response = await fetch(new URL('meta.json', origin))
...@@ -18,7 +15,7 @@ export async function getMergedMetadata ({ version }) { ...@@ -18,7 +15,7 @@ export async function getMergedMetadata ({ version }) {
} }
})) }))
newMetadata.push({ metadata.push({
id: 'ui-middleware', id: 'ui-middleware',
name: 'UI Middleware', name: 'UI Middleware',
buildDate: process.env.BUILD_TIMESTAMP, buildDate: process.env.BUILD_TIMESTAMP,
...@@ -27,7 +24,13 @@ export async function getMergedMetadata ({ version }) { ...@@ -27,7 +24,13 @@ export async function getMergedMetadata ({ version }) {
}) })
// only return when contains data // only return when contains data
const filtered = newMetadata.filter(Boolean) return metadata.filter(Boolean)
await cache.set(getRedisKey({ version, name: 'mergedMetadata' }), JSON.stringify(filtered)) }
return filtered
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)
} }
src/middlewares/load-from-cache.js deleted 100644 → 0
+ 0
− 20
View file @ 451c9023
import { loadFromCache } from '../files.js'
export default async function (req, res, next) {
try {
const data = await loadFromCache({
path: req.path,
version: res.version
})
if (!data) return
const { body, headers, sha256Sum } = data
res.body = body
res.headers = headers
res.locals.sha256Sum = sha256Sum
res.cache = false
} catch (err) {
next(err)
} finally {
next()
}
}
src/middlewares/save-to-cache.js deleted 100644 → 0
+ 0
− 20
View file @ 451c9023
import { saveToCache } from '../files.js'
import { logger } from '../logger.js'
export default async function (req, res, next) {
try {
if (!res.body) return
if (res.cache === false) return
saveToCache({
version: res.version,
path: req.path,
body: res.body,
headers: res.headers,
sha256Sum: res.locals.sha256Sum
}).catch(() => logger.error)
} catch (err) {
next(err)
} finally {
next()
}
}
src/middlewares/load-from-server.js → src/middlewares/serve-files.js
+ 28
− 0
View file @ 43b4dd6a
import { fetchFileWithHeaders, loadFromCache, saveAsyncToCache } from '../files.js'
import { NotFoundError } from '../errors.js' import { NotFoundError } from '../errors.js'
import { fetchFileWithHeaders } from '../files.js'
import createError from 'http-errors' import createError from 'http-errors'
export default async function (req, res, next) { export default async function (req, res, next) {
try { try {
if (res.body) return if (req.method !== 'GET') return next()
const path = req.path === '/' ? '/index.html' : req.path const version = res.version
const { body, headers, sha256Sum } = await fetchFileWithHeaders({ path, version: res.version }) let data = await loadFromCache({
res.body = body path: req.path,
res.headers = headers version
})
if (!data) {
const path = req.path === '/' ? '/index.html' : req.path
data = await saveAsyncToCache({ version, path, promise: fetchFileWithHeaders({ path, version }) })
}
const { body, headers, sha256Sum } = data
res.set(headers)
res.locals.sha256Sum = sha256Sum res.locals.sha256Sum = sha256Sum
res.send(body)
} catch (err) { } catch (err) {
// response might be an aggregate error. therefore need to check all errors
const errors = err.errors || [err] const errors = err.errors || [err]
const fileNotFound = errors.reduce((memo, error) => memo && error instanceof NotFoundError, true) const fileNotFound = errors.reduce((memo, error) => memo && error instanceof NotFoundError, true)
if (fileNotFound) next(createError(404, 'File does not exist.')) if (fileNotFound) next(createError(404, 'File does not exist.'))
else next(err) else next(err)
} finally {
next()
} }
} }
src/routes/manifests.js
+ 1
− 4
View file @ 43b4dd6a
...@@ -6,12 +6,9 @@ const router = new Router() ...@@ -6,12 +6,9 @@ const router = new Router()
router.get('/manifests', async function (req, res, next) { router.get('/manifests', async function (req, res, next) {
try { try {
if (res.body) return if (res.body) return
res.body = await getOxManifests({ version: res.version }) res.send(await getOxManifests({ version: res.version }))
res.headers = { 'content-type': 'application/json' }
} catch (err) { } catch (err) {
next(err) next(err)
} finally {
next()
} }
}) })
......
src/routes/metadata.js
+ 1
− 4
View file @ 43b4dd6a
...@@ -5,12 +5,9 @@ const router = new Router() ...@@ -5,12 +5,9 @@ const router = new Router()
router.get('/meta', async (req, res, next) => { router.get('/meta', async (req, res, next) => {
try { try {
res.body = await getMergedMetadata({ version: res.version }) res.send(await getMergedMetadata({ version: res.version }))
res.headers = { 'content-type': 'application/json' }
} catch (err) { } catch (err) {
next(err) next(err)
} finally {
next()
} }
}) })
......
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment