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

feat(gatsby-core-utils): create proper mutex #34761

Merged
merged 7 commits into from
Feb 16, 2022
Merged

feat(gatsby-core-utils): create proper mutex #34761

merged 7 commits into from
Feb 16, 2022

Conversation

wardpeet
Copy link
Contributor

@wardpeet wardpeet commented Feb 8, 2022

Description

Adding a multi-process mutex that works 100% of the time by leveraging LMDB. LMDB's ifNoExists helps us with thread/process safety.

The API supports a createMutex function that expects a key. This function returns an object with two functions acquire and release. Acquire will wait for the mutex to be available and release will release the current mutex. This is a powerful mechanism.

For this change, lmdb 2.0 is necessary as lmdb 1.0 didn't make sure only one proc went through. So I try to get the lmdb instance from gatsby. If it's not available, it will use its own.

A proper integration test will be written with fetchRemoteFileNode in a follow-up

@wardpeet wardpeet added the topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) label Feb 8, 2022
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 8, 2022
@wardpeet wardpeet removed the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 8, 2022
packages/gatsby-core-utils/package.json Show resolved Hide resolved
packages/gatsby-core-utils/src/mutex.ts Outdated Show resolved Hide resolved
packages/gatsby-core-utils/src/mutex.ts Outdated Show resolved Hide resolved
packages/gatsby/src/services/initialize.ts Outdated Show resolved Hide resolved
packages/gatsby/src/services/initialize.ts Show resolved Hide resolved
import { createMutex } from "../mutex"
import * as storage from "../utils/get-storage"

jest.spyOn(storage, `getDatabaseDir`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we throw this in a beforeEach and reset it within an afterEach?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure what the point is here? Only thing spyOn does is make it possible to listen to it.

packages/gatsby-core-utils/src/mutex.ts Outdated Show resolved Hide resolved
export async function releaseAllMutexes(): Promise<void> {
const storage = getStorage(getDatabaseDir())

await storage.mutex.clearAsync()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you view this being used for? It feels like it could be a footgun for plugin creators, and feel iffy about exposing it.

Copy link
Contributor Author

@wardpeet wardpeet Feb 13, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we'll use it in gatsby core, that's the only reason. I can't clear mutexes other wise from other builds, let's say a previous build crashed and mutexes are still alive, this list can grow until someone does gatsby clean.

Seems like I need to update this branch to latest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

@LekoArts LekoArts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants