Skip to content

Commit

Permalink
fix: Serialize access to pod logs (#53)
Browse files Browse the repository at this point in the history
  • Loading branch information
meyfa authored Jul 14, 2024
1 parent 24d0334 commit 9cbc552
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 21 deletions.
23 changes: 16 additions & 7 deletions backend/src/api/pod-logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { authenticateSession } from '../auth/common.js'
import { prettifyLogs } from '../renovate/prettify-logs.js'
import { enums, optional, type } from 'superstruct'
import { WeakCache } from '../util/cache.js'
import { Mutex } from '../util/mutex.js'

export interface PodLogsRoute {
Reply: string
Expand All @@ -17,8 +18,10 @@ const logsQuerystringSchema = type({
export const podLogsRoute = ({ logsController }: Controllers): FastifyPluginAsync => async (app) => {
app.addHook('preValidation', authenticateSession())

// Prettifying is an expensive operation. Cache the results.
// Prettifying is an expensive operation. Cache the results, and only allow one prettification at a time.
// The latter helps if multiple clients request the same logs at the same time since no
const prettyLogsCache = new WeakCache<{ data: string }>(5000)
const prettyLogsMutex = new Mutex()

app.get<PodLogsRoute & {
Params: {
Expand All @@ -38,12 +41,18 @@ export const podLogsRoute = ({ logsController }: Controllers): FastifyPluginAsyn
return await notFound(reply)
}
if (query.pretty === 'true') {
const result = await prettyLogsCache.lazyCompute([request.params.namespace, request.params.name], async () => {
return {
data: await prettifyLogs(logs)
}
})
return result?.data
let unlock
try {
unlock = await prettyLogsMutex.lock()
const result = await prettyLogsCache.lazyCompute([request.params.namespace, request.params.name], async () => {
return {
data: await prettifyLogs(logs)
}
})
return result?.data
} finally {
unlock?.()
}
}
return logs
})
Expand Down
38 changes: 24 additions & 14 deletions backend/src/controllers/logs.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { KubernetesApi } from '../kubernetes/api.js'
import { PodController } from './pod.js'
import { WeakCache } from '../util/cache.js'
import { Mutex } from '../util/mutex.js'

export class LogsController {
// The log strings may be large, so we use a weak cache to allow them to be garbage collected.
// However, it's impossible to create a WeakRef to a string, so we wrap the string in an object.
private readonly cache = new WeakCache<{ data: string }>(5000)

// Only allow fetching one log at a time.
private readonly mutex = new Mutex()

constructor (
private readonly k8s: KubernetesApi,
private readonly podController: PodController
Expand All @@ -17,20 +21,26 @@ export class LogsController {
namespace: string
name: string
}): Promise<string | undefined> {
// Input must relate to an existing pod
const podItem = await this.podController.findPod(pod)
if (podItem == null) {
return undefined
}
// Requesting logs for a pod that is starting is invalid and will result in a 400 error.
if (podItem.status?.phase === 'Pending' || podItem.status?.phase === 'Unknown') {
return undefined
}
const result = await this.cache.lazyCompute([pod.namespace, pod.name], async () => {
return {
data: await this.k8s.getPodLogs(pod)
let unlock
try {
unlock = await this.mutex.lock()
// Input must relate to an existing pod
const podItem = await this.podController.findPod(pod)
if (podItem == null) {
return undefined
}
})
return result?.data
// Requesting logs for a pod that is starting is invalid and will result in a 400 error.
if (podItem.status?.phase === 'Pending' || podItem.status?.phase === 'Unknown') {
return undefined
}
const result = await this.cache.lazyCompute([pod.namespace, pod.name], async () => {
return {
data: await this.k8s.getPodLogs(pod)
}
})
return result?.data
} finally {
unlock?.()
}
}
}

0 comments on commit 9cbc552

Please sign in to comment.