Skip to content

Commit

Permalink
feat(gatsby): enable Jobs API v2 (#19858)
Browse files Browse the repository at this point in the history
Added a new action called createJobV2 to support the new api. createJob is still available to keep backward-compatibility. I'll add a deprecation message when creatJobV2 is fully operational.

The createJobV2 needs a job object that contains 4 arguments name, inputPaths, outputDir & args. These args are used to create a unique content digest to make sure a job is deterministic.

InputPaths are converted into relative paths before sending it to the worker as they need to be filesystem agnostic.

More info on why can be found in #19831
  • Loading branch information
wardpeet authored Jan 21, 2020
1 parent 65a6833 commit 039c601
Show file tree
Hide file tree
Showing 28 changed files with 1,210 additions and 27 deletions.
1 change: 1 addition & 0 deletions e2e-tests/production-runtime/gatsby-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ module.exports = {
icon: `src/images/gatsby-icon.png`, // This path is relative to the root of the site.
},
},
`gatsby-plugin-local-worker`,
].concat(process.env.TEST_PLUGIN_OFFLINE ? [`gatsby-plugin-offline`] : []),
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
const path = require(`path`)

exports.onPostBootstrap = async ({ actions, store }) => {
const result = await actions.createJobV2({
name: `TEST_JOB`,
inputPaths: [],
outputDir: path.join(store.getState().program.directory, `./public`),
args: {
result: `hello`,
},
})

if (result.result !== `hello`) {
throw new Error(`the result of our worker is wrong`)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
const sleep = timeout => new Promise(resolve => setTimeout(resolve, timeout))

exports.TEST_JOB = async ({ args }) => {
await sleep(5000)

return {
result: args.result,
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
// noop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "gatsby-plugin-local-worker",
"version": "1.0.0"
}
18 changes: 17 additions & 1 deletion packages/gatsby/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,12 @@ export interface Actions {

/** @see https://www.gatsbyjs.org/docs/actions/#createPage */
createPage<TContext = Record<string, unknown>>(
args: { path: string; matchPath?: string; component: string; context: TContext },
args: {
path: string
matchPath?: string
component: string
context: TContext
},
plugin?: ActionPlugin,
option?: ActionOptions
): void
Expand Down Expand Up @@ -878,6 +883,17 @@ export interface Actions {
plugin?: ActionPlugin
): void

/** @see https://www.gatsbyjs.org/docs/actions/#createJobV2 */
createJobV2(
job: {
name: string
inputPaths: string[]
outputDir: string
args: Record<string, unknown>
},
plugin?: ActionPlugin
): Promise<unknown>

/** @see https://www.gatsbyjs.org/docs/actions/#setJob */
setJob(
job: Record<string, unknown> & { id: string },
Expand Down
2 changes: 2 additions & 0 deletions packages/gatsby/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"graphql": "^14.5.8",
"graphql-compose": "^6.3.7",
"graphql-playground-middleware-express": "^1.7.12",
"hasha": "^5.1.0",
"invariant": "^2.2.4",
"is-relative": "^1.0.0",
"is-relative-url": "^3.0.0",
Expand All @@ -104,6 +105,7 @@
"opentracing": "^0.14.4",
"optimize-css-assets-webpack-plugin": "^5.0.3",
"parseurl": "^1.3.3",
"p-defer": "^3.0.0",
"physical-cpu-count": "^2.0.0",
"pnp-webpack-plugin": "^1.5.0",
"postcss-flexbugs-fixes": "^3.3.1",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`remove-stale-jobs should enqueue pending jobs 1`] = `
Array [
undefined,
]
`;

exports[`remove-stale-jobs should remove stale jobs from complete cache 1`] = `
Array [
Object {
"payload": Object {
"contentDigest": "1234",
},
"plugin": undefined,
"traceId": undefined,
"type": "REMOVE_STALE_JOB_V2",
},
]
`;

exports[`remove-stale-jobs should remove stale jobs from pending cache 1`] = `
Array [
Object {
"payload": Object {
"contentDigest": "1234",
},
"plugin": undefined,
"traceId": undefined,
"type": "REMOVE_STALE_JOB_V2",
},
]
`;
86 changes: 86 additions & 0 deletions packages/gatsby/src/bootstrap/__tests__/remove-stale-jobs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
jest.mock(`../../utils/jobs-manager`)

const { isJobStale } = require(`../../utils/jobs-manager`)
const { internalActions, publicActions } = require(`../../redux/actions`)

jest.spyOn(internalActions, `removeStaleJob`)

const removeStaleJobs = require(`../remove-stale-jobs`)

describe(`remove-stale-jobs`, () => {
let state

beforeEach(() => {
state = {
jobsV2: {
complete: new Map(),
incomplete: new Map(),
},
}

publicActions.createJobV2 = jest.fn()
internalActions.removeStaleJob.mockClear()
})

it(`should remove stale jobs from complete cache`, () => {
const job = {
inputPaths: [`/src/myfile.js`],
}

state.jobsV2.complete.set(`1234`, job)

isJobStale.mockReturnValue(true)

expect(removeStaleJobs(state)).toMatchSnapshot()
expect(internalActions.removeStaleJob).toHaveBeenCalledTimes(1)
expect(internalActions.removeStaleJob).toHaveBeenCalledWith(`1234`)
expect(publicActions.createJobV2).not.toHaveBeenCalled()
})

it(`should remove stale jobs from pending cache`, () => {
const data = {
job: {
inputPaths: [`/src/myfile.js`],
contentDigest: `1234`,
},
plugin: {
name: `test`,
version: `1.0.0`,
},
}

state.jobsV2.incomplete.set(`1234`, data)

isJobStale.mockReturnValue(true)

expect(removeStaleJobs(state)).toMatchSnapshot()
expect(internalActions.removeStaleJob).toHaveBeenCalledTimes(1)
expect(internalActions.removeStaleJob).toHaveBeenCalledWith(`1234`)
expect(publicActions.createJobV2).not.toHaveBeenCalled()
})

it(`should enqueue pending jobs`, () => {
const data = {
job: {
inputPaths: [`/src/myfile.js`],
contentDigest: `1234`,
},
plugin: {
name: `test`,
version: `1.0.0`,
},
}

state.jobsV2.incomplete.set(`1234`, data)

isJobStale.mockReturnValue(false)

expect(removeStaleJobs(state)).toMatchSnapshot()
expect(internalActions.removeStaleJob).toHaveBeenCalledTimes(0)
expect(publicActions.createJobV2).toHaveBeenCalledTimes(1)
expect(publicActions.createJobV2).toHaveBeenCalledWith(
data.job,
data.plugin
)
})
})
6 changes: 5 additions & 1 deletion packages/gatsby/src/bootstrap/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const report = require(`gatsby-cli/lib/reporter`)
const getConfigFile = require(`./get-config-file`)
const tracer = require(`opentracing`).globalTracer()
const preferDefault = require(`./prefer-default`)
const removeStaleJobs = require(`./remove-stale-jobs`)
// Add `util.promisify` polyfill for old node versions
require(`util.promisify/shim`)()

Expand Down Expand Up @@ -160,6 +161,9 @@ module.exports = async (args: BootstrapArgs) => {

activity.end()

// run stale jobs
store.dispatch(removeStaleJobs(store.getState()))

activity = report.activityTimer(`load plugins`, { parentSpan: bootstrapSpan })
activity.start()
const flattenedPlugins = await loadPlugins(config, program.directory)
Expand Down Expand Up @@ -229,7 +233,7 @@ module.exports = async (args: BootstrapArgs) => {
.createHash(`md5`)
.update(JSON.stringify(pluginVersions.concat(hashes)))
.digest(`hex`)
let state = store.getState()
const state = store.getState()
const oldPluginsHash = state && state.status ? state.status.PLUGINS_HASH : ``

// Check if anything has changed. If it has, delete the site's .cache
Expand Down
25 changes: 25 additions & 0 deletions packages/gatsby/src/bootstrap/remove-stale-jobs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
const { isJobStale } = require(`../utils/jobs-manager`)
const { publicActions, internalActions } = require(`../redux/actions`)

module.exports = state => {
const actions = []

// If any of our finished jobs are stale we remove them to keep our cache small
state.jobsV2.complete.forEach((job, contentDigest) => {
if (isJobStale(job)) {
actions.push(internalActions.removeStaleJob(contentDigest))
}
})

// If any of our pending jobs do not have an existing inputPath or the inputPath changed
// we remove it from the queue as they would fail anyway
state.jobsV2.incomplete.forEach(({ job, plugin }) => {
if (isJobStale(job)) {
actions.push(internalActions.removeStaleJob(job.contentDigest))
} else {
actions.push(publicActions.createJobV2(job, plugin))
}
})

return actions
}
22 changes: 19 additions & 3 deletions packages/gatsby/src/commands/build.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ const queryUtil = require(`../query`)
const appDataUtil = require(`../utils/app-data`)
const WorkerPool = require(`../utils/worker/pool`)
const { structureWebpackErrors } = require(`../utils/webpack-error-utils`)
const {
waitUntilAllJobsComplete: waitUntilAllJobsV2Complete,
} = require(`../utils/jobs-manager`)

type BuildArgs = {
directory: string,
Expand All @@ -25,8 +28,8 @@ type BuildArgs = {
openTracingConfigFile: string,
}

const waitJobsFinished = () =>
new Promise((resolve, reject) => {
const waitUntilAllJobsComplete = () => {
const jobsV1Promise = new Promise(resolve => {
const onEndJob = () => {
if (store.getState().jobs.active.length === 0) {
resolve()
Expand All @@ -37,6 +40,12 @@ const waitJobsFinished = () =>
onEndJob()
})

return Promise.all([
jobsV1Promise,
waitUntilAllJobsV2Complete(),
]).then(() => {})
}

module.exports = async function build(program: BuildArgs) {
const publicDir = path.join(program.directory, `public`)
initTracer(program.openTracingConfigFile)
Expand Down Expand Up @@ -128,9 +137,13 @@ module.exports = async function build(program: BuildArgs) {
`BOOTSTRAP_QUERY_RUNNING_FINISHED`
)

await waitJobsFinished()
await db.saveState()

await waitUntilAllJobsComplete()

// we need to save it again to make sure our latest state has been saved
await db.saveState()

const pagePaths = [...store.getState().pages.keys()]
activity = report.createProgress(
`Building static HTML for pages`,
Expand Down Expand Up @@ -176,6 +189,9 @@ module.exports = async function build(program: BuildArgs) {
parentSpan: buildSpan,
})

// Make sure we saved the latest state so we have all jobs cached
await db.saveState()

report.info(`Done building in ${process.uptime()} sec`)

buildSpan.finish()
Expand Down
42 changes: 26 additions & 16 deletions packages/gatsby/src/commands/develop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ import {
structureWebpackErrors,
} from "../utils/webpack-error-utils"

import { waitUntilAllJobsComplete as waitUntilAllJobsV2Complete } from "../utils/jobs-manager"

interface ICert {
keyPath: string
certPath: string
Expand All @@ -75,6 +77,24 @@ interface IProgram {
ssl?: ICert
}

const waitUntilAllJobsComplete = (): Promise<void> => {
const jobsV1Promise = new Promise(resolve => {
const onEndJob = (): void => {
if (store.getState().jobs.active.length === 0) {
resolve()
emitter.off(`END_JOB`, onEndJob)
}
}
emitter.on(`END_JOB`, onEndJob)
onEndJob()
})

return Promise.all([
jobsV1Promise,
waitUntilAllJobsV2Complete(),
]).then(() => {})
}

// const isInteractive = process.stdout.isTTY

// Watch the static directory and copy files to public as they're added or
Expand All @@ -88,18 +108,6 @@ onExit(() => {
telemetry.trackCli(`DEVELOP_STOP`)
})

const waitJobsFinished = (): Promise<void> =>
new Promise(resolve => {
const onEndJob = (): void => {
if (store.getState().jobs.active.length === 0) {
resolve()
emitter.off(`END_JOB`, onEndJob)
}
}
emitter.on(`END_JOB`, onEndJob)
onEndJob()
})

type ActivityTracker = any // TODO: Replace this with proper type once reporter is typed

interface IServer {
Expand Down Expand Up @@ -421,7 +429,9 @@ module.exports = async (program: IProgram): Promise<void> => {
`BOOTSTRAP_QUERY_RUNNING_FINISHED`
)

await waitJobsFinished()
await db.saveState()

await waitUntilAllJobsComplete()
requiresWriter.startListener()
db.startAutosave()
queryUtil.startListeningToDevelopQueue()
Expand Down Expand Up @@ -457,9 +467,9 @@ module.exports = async (program: IProgram): Promise<void> => {
})

const isUnspecifiedHost = host === `0.0.0.0` || host === `::`
let prettyHost = host,
lanUrlForConfig,
lanUrlForTerminal
let prettyHost = host
let lanUrlForConfig
let lanUrlForTerminal
if (isUnspecifiedHost) {
prettyHost = `localhost`

Expand Down
Loading

0 comments on commit 039c601

Please sign in to comment.