Skip to content

Commit

Permalink
next-dev: change cluster usage to child process + stabilise inspect p…
Browse files Browse the repository at this point in the history
…ort (#45745

Fixes #45122 and #44817

NEXT-447

This PR:
- changes the usage of the cluster module in `next dev` to use
`child_process` instead. This is mostly the same thing and helps
alleviate issues with libraries that rely on `Cluster.isMaster`.
- fixes the implementation also to check if the `--inspect` option is
used to then manually attribute the correct port, which is whatever the
current port is, plus one. With the previous cluster implementation, it
always increased automatically every time the server died.
- adds back the warning log I had removed by mistake to inform the user
that the server is restarting.


<!--
Thanks for opening a PR! Your contribution is much appreciated.
To make sure your PR is handled as smoothly as possible we request that
you follow the checklist sections below.
Choose the right checklist for the change(s) that you're making:
-->

## Bug

- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the
feature request has been accepted for implementation before opening a
PR.
- [ ] Related issues linked using `fixes #number`
- [ ]
[e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs)
tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
  • Loading branch information
feedthejim committed Feb 10, 2023
1 parent 484f472 commit be5fd6c
Show file tree
Hide file tree
Showing 2 changed files with 104 additions and 57 deletions.
149 changes: 95 additions & 54 deletions packages/next/src/cli/next-dev.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#!/usr/bin/env node
import arg from 'next/dist/compiled/arg/index.js'
import { startServer } from '../server/lib/start-server'
import { startServer, WORKER_SELF_EXIT_CODE } from '../server/lib/start-server'
import { getPort, printAndExit } from '../server/lib/utils'
import * as Log from '../build/output/log'
import { startedDevelopmentServer } from '../build/output'
Expand All @@ -13,20 +13,23 @@ import type { NextConfig } from '../../types'
import type { NextConfigComplete } from '../server/config-shared'
import { traceGlobals } from '../trace/shared'
import { isIPv6 } from 'net'
import cluster from 'cluster'
import { ChildProcess, fork } from 'child_process'
import { Telemetry } from '../telemetry/storage'
import loadConfig from '../server/config'
import { findPagesDir } from '../lib/find-pages-dir'
import { fileExists } from '../lib/file-exists'
import Watchpack from 'next/dist/compiled/watchpack'
import stripAnsi from 'next/dist/compiled/strip-ansi'
import { warn } from '../build/output/log'

let isTurboSession = false
let sessionStopHandled = false
let sessionStarted = Date.now()
let dir: string
let unwatchConfigFiles: () => void

const isChildProcess = !!process.env.__NEXT_DEV_CHILD_PROCESS

const handleSessionStop = async () => {
if (sessionStopHandled) return
sessionStopHandled = true
Expand Down Expand Up @@ -81,7 +84,7 @@ const handleSessionStop = async () => {
process.exit(0)
}

if (cluster.isMaster) {
if (!isChildProcess) {
process.on('SIGINT', handleSessionStop)
process.on('SIGTERM', handleSessionStop)
} else {
Expand Down Expand Up @@ -432,16 +435,75 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
// we're using a sub worker to avoid memory leaks. When memory usage exceeds 90%, we kill the worker and restart it.
// this is a temporary solution until we can fix the memory leaks.
// the logic for the worker killing itself is in `packages/next/server/lib/start-server.ts`
if (!process.env.__NEXT_DISABLE_MEMORY_WATCHER && cluster.isMaster) {
if (!process.env.__NEXT_DISABLE_MEMORY_WATCHER && !isChildProcess) {
let config: NextConfig
let childProcess: ChildProcess | null = null

const isDebugging = process.execArgv.some((localArg) =>
localArg.startsWith('--inspect')
)

const isDebuggingWithBrk = process.execArgv.some((localArg) =>
localArg.startsWith('--inspect-brk')
)

const debugPort = (() => {
const debugPortStr = process.execArgv
.find(
(localArg) =>
localArg.startsWith('--inspect') ||
localArg.startsWith('--inspect-brk')
)
?.split('=')[1]
return debugPortStr ? parseInt(debugPortStr, 10) : 9229
})()

if (isDebugging || isDebuggingWithBrk) {
warn(
`the --inspect${
isDebuggingWithBrk ? '-brk' : ''
} option was detected, the Next.js server should be inspected at port ${
debugPort + 1
}.`
)
}

const genExecArgv = () => {
const execArgv = process.execArgv.filter((localArg) => {
return (
!localArg.startsWith('--inspect') &&
!localArg.startsWith('--inspect-brk')
)
})

const setupFork = (env?: Parameters<typeof cluster.fork>[0]) => {
if (isDebugging || isDebuggingWithBrk) {
execArgv.push(
`--inspect${isDebuggingWithBrk ? '-brk' : ''}=${debugPort + 1}`
)
}

return execArgv
}

const setupFork = (env?: NodeJS.ProcessEnv, newDir?: string) => {
const startDir = dir
const [, script, ...nodeArgs] = process.argv
let shouldFilter = false
cluster.fork({
...env,
FORCE_COLOR: '1',
})
childProcess = fork(
newDir ? script.replace(startDir, newDir) : script,
nodeArgs,
{
env: {
...(env ? env : process.env),
FORCE_COLOR: '1',
__NEXT_DEV_CHILD_PROCESS: '1',
},
// @ts-ignore TODO: remove ignore when types are updated
windowsHide: true,
stdio: ['ipc', 'pipe', 'pipe'],
execArgv: genExecArgv(),
}
)

// since errors can start being logged from the fork
// before we detect the project directory rename
Expand Down Expand Up @@ -473,40 +535,27 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
process[fd].write(chunk)
}

for (const workerId in cluster.workers) {
cluster.workers[workerId]?.process.stdout?.on('data', (chunk) => {
filterForkErrors(chunk, 'stdout')
})
cluster.workers[workerId]?.process.stderr?.on('data', (chunk) => {
filterForkErrors(chunk, 'stderr')
})
}
}

const handleClusterExit = () => {
const callback = async (worker: cluster.Worker) => {
// ignore if we killed the worker
if ((worker as any).killed) return
childProcess?.stdout?.on('data', (chunk) => {
filterForkErrors(chunk, 'stdout')
})
childProcess?.stderr?.on('data', (chunk) => {
filterForkErrors(chunk, 'stderr')
})

// TODO: we should track how many restarts are
// occurring and how long in-between them
if (worker.exitedAfterDisconnect) {
const callback = async (code: number | null) => {
if (code === WORKER_SELF_EXIT_CODE) {
setupFork()
} else if (!sessionStopHandled) {
await handleSessionStop()
process.exit(1)
}
}
cluster.addListener('exit', callback)
return () => cluster.removeListener('exit', callback)
childProcess?.addListener('exit', callback)
return () => childProcess?.removeListener('exit', callback)
}
let clusterExitUnsub = handleClusterExit()
// x-ref: https://nodejs.org/api/cluster.html#clustersettings
// @ts-expect-error type is incorrect
cluster.settings.windowsHide = true
cluster.settings.stdio = ['ipc', 'pipe', 'pipe']

setupFork()
let childProcessExitUnsub = setupFork()

config = await loadConfig(
PHASE_DEVELOPMENT_SERVER,
dir,
Expand All @@ -516,27 +565,19 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
)

const handleProjectDirRename = (newDir: string) => {
clusterExitUnsub()

for (const workerId in cluster.workers) {
try {
// @ts-expect-error custom field
cluster.workers[workerId].killed = true
cluster.workers[workerId]!.process.kill('SIGKILL')
} catch (_) {}
}
childProcessExitUnsub()
childProcess?.kill()
process.chdir(newDir)
// @ts-expect-error type is incorrect
cluster.settings.cwd = newDir
cluster.settings.exec = cluster.settings.exec?.replace(dir, newDir)
setupFork({
...Object.keys(process.env).reduce((newEnv, key) => {
newEnv[key] = process.env[key]?.replace(dir, newDir)
return newEnv
}, {} as typeof process.env),
NEXT_PRIVATE_DEV_DIR: newDir,
})
clusterExitUnsub = handleClusterExit()
childProcessExitUnsub = setupFork(
{
...Object.keys(process.env).reduce((newEnv, key) => {
newEnv[key] = process.env[key]?.replace(dir, newDir)
return newEnv
}, {} as typeof process.env),
NEXT_PRIVATE_DEV_DIR: newDir,
},
newDir
)
}
const parentDir = path.join('/', dir, '..')
const watchedEntryLength = parentDir.split('/').length + 1
Expand Down
12 changes: 9 additions & 3 deletions packages/next/src/server/lib/start-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ import { warn } from '../../build/output/log'
import http from 'http'
import next from '../next'
import { isIPv6 } from 'net'
import cluster from 'cluster'
import v8 from 'v8'
const isChildProcess = !!process.env.__NEXT_DEV_CHILD_PROCESS

interface StartServerOptions extends NextServerOptions {
allowRetry?: boolean
keepAliveTimeout?: number
}

export const WORKER_SELF_EXIT_CODE = 77

const MAXIMUM_HEAP_SIZE_ALLOWED =
(v8.getHeapStatistics().heap_size_limit / 1024 / 1024) * 0.9

Expand All @@ -20,10 +22,14 @@ export function startServer(opts: StartServerOptions) {
const server = http.createServer((req, res) => {
return requestHandler(req, res).finally(() => {
if (
cluster.worker &&
isChildProcess &&
process.memoryUsage().heapUsed / 1024 / 1024 > MAXIMUM_HEAP_SIZE_ALLOWED
) {
cluster.worker.kill()
warn(
'The server is running out of memory, restarting to free up memory.'
)
server.close()
process.exit(WORKER_SELF_EXIT_CODE)
}
})
})
Expand Down

0 comments on commit be5fd6c

Please sign in to comment.