-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
Conversation
import { createMutex } from "../mutex" | ||
import * as storage from "../utils/get-storage" | ||
|
||
jest.spyOn(storage, `getDatabaseDir`) |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
export async function releaseAllMutexes(): Promise<void> { | ||
const storage = getStorage(getDatabaseDir()) | ||
|
||
await storage.mutex.clearAsync() |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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 functionsacquire
andrelease.
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