Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Serialize access to pod logs #53

Merged
merged 1 commit into from
Jul 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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?.()
}
}
}