Add file operation coordinator to the registry - #13628
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
a5b665f to
a56a2b6
Compare
a5b665f to
a56a2b6
Compare
a56a2b6 to
7f2804b
Compare
7f2804b to
8e507b3
Compare
8e507b3 to
df639a3
Compare
df639a3 to
fe93250
Compare
fe93250 to
4e798cc
Compare
4e798cc to
34f6fca
Compare
cf488ce to
b18ae25
Compare
b18ae25 to
4b812e0
Compare
4b812e0 to
1628383
Compare
1628383 to
9d5a48d
Compare
franknoirot
left a comment
There was a problem hiding this comment.
Okay I know that adding both fast-check and Effect in this stack is a lot but I hope I can convince y'all it's worth it here.
| } | ||
|
|
||
| /** Build the preferred filename or its numbered collision variant. */ | ||
| export function fileNameCandidate(name: FileNameParts, suffix: number): string { |
There was a problem hiding this comment.
I know this is duplicative of our path helpers, but idk I kinda want this to stand alone? Or I need our path libraries to have less baggage and use only pure functions
| } from '@src/lib/fileSystem/fileOperations' | ||
|
|
||
| /** Promise facade for coordinated project-directory and file operations. */ | ||
| export interface FileOperationsRegistryService { |
There was a problem hiding this comment.
This is where to start: this service should have everything we need to interact with a file operation queue in one shared place, so let me know if anything looks wrong.
| const runRuntimePromise = async <A, E, R extends FileOperations>( | ||
| program: Effect.Effect<A, E, R>, | ||
| onFailure?: (error: E) => void | ||
| ): Promise<A> => { | ||
| const result = await runtime.runPromise(program.pipe(Effect.either)) | ||
| if (Either.isLeft(result)) { | ||
| onFailure?.(result.left) | ||
| return Promise.reject(result.left) | ||
| } | ||
| return result.right | ||
| } |
There was a problem hiding this comment.
Effect calls nested operations "programs" and they can compose really nicely in reusable ways, since it's just a bougie functional programming library. Effect.either is a reusable program they export that picks left (unsuccessful) or right (successful) outcomes, like lambda calculus.
Each of the service methods is just wrapped in this so they all handle rejection and piping in the same way.
| program: Effect.Effect<A, FileSystemError, FileOperations> | ||
| ) => | ||
| runRuntimePromise(program, (error) => | ||
| reportFileOperationsError(operation, error) |
There was a problem hiding this comment.
This is where the unsuccessful case's return type is established, so Effect can tell the TypeScript type checker what error types are possible for each of these service methods!
| while (true) { | ||
| const parent = backing.dirname(current) | ||
| if (parent === current) { | ||
| break | ||
| } | ||
|
|
||
| if (!modesByPath.has(parent)) { | ||
| modesByPath.set(parent, 'shared') | ||
| } |
There was a problem hiding this comment.
This is where we detect descendants for lock status
| const reserveLock = (locks: typeof pathLocks, path: string) => | ||
| SynchronizedRef.modifyEffect(locks, (entries) => { | ||
| const existing = entries.get(path) | ||
| if (existing) { | ||
| const next = new Map(entries) | ||
| next.set(path, { ...existing, users: existing.users + 1 }) | ||
| return Effect.succeed([existing.semaphore, next] as const) | ||
| } | ||
|
|
||
| return Effect.makeSemaphore(EXCLUSIVE_LOCK_PERMITS).pipe( | ||
| Effect.map((semaphore) => { | ||
| const next = new Map(entries) | ||
| next.set(path, { semaphore, users: 1 }) | ||
| return [semaphore, next] as const | ||
| }) | ||
| ) | ||
| }) | ||
|
|
||
| const releaseLock = (locks: typeof pathLocks, path: string) => | ||
| SynchronizedRef.update(locks, (entries) => { | ||
| const existing = entries.get(path) | ||
| if (!existing) { | ||
| return entries | ||
| } | ||
|
|
||
| const next = new Map(entries) | ||
| if (existing.users === 1) { | ||
| next.delete(path) | ||
| } else { | ||
| next.set(path, { ...existing, users: existing.users - 1 }) | ||
| } | ||
| return next | ||
| }) | ||
|
|
||
| const withLocks = <A, E, R>( | ||
| locks: typeof pathLocks, | ||
| requirements: readonly PathLockRequirement[], | ||
| operation: Effect.Effect<A, E, R>, | ||
| index = 0 | ||
| ): Effect.Effect<A, E, R> => { | ||
| const requirement = requirements[index] | ||
| if (!requirement) { | ||
| return operation | ||
| } | ||
|
|
||
| return Effect.acquireUseRelease( | ||
| reserveLock(locks, requirement.path), | ||
| (semaphore) => | ||
| semaphore.withPermits( | ||
| requirement.mode === 'exclusive' | ||
| ? EXCLUSIVE_LOCK_PERMITS | ||
| : SHARED_LOCK_PERMITS | ||
| )(withLocks(locks, requirements, operation, index + 1)), | ||
| () => releaseLock(locks, requirement.path) | ||
| ) | ||
| } |
There was a problem hiding this comment.
This right here is why I reached for Effect. Locking and coordination is hard as fuck, but this library lets us compose those policies that, at least at the end of the pipeline, are pretty readable, and way easier to debug. Plus it has built-in notions of parallelization and semaphores.
| const coordinateMutation = <A, E, R>( | ||
| paths: readonly string[], | ||
| operation: Effect.Effect<A, E, R>, | ||
| membershipPaths = paths.map((path) => backing.dirname(path)) | ||
| ) => | ||
| trackMutation( | ||
| withPathLocks( | ||
| pathLockRequirements(backing, paths), | ||
| withDirectoryMembershipLocks(membershipPaths, 'exclusive', operation) | ||
| ) | ||
| ) |
There was a problem hiding this comment.
See now we can see here that mutations are coordinated by applying exclusive membership locks to all the necessary paths.
| return FileOperations.of({ | ||
| pending: SubscriptionRef.get(pending), | ||
| pendingChanges: pending.changes, | ||
| stat: (path) => coordinateRead(path, fileSystem.stat(path)), | ||
| canReadWrite: (path) => | ||
| coordinateRead(path, fileSystem.canReadWrite(path)), | ||
| exists: (path) => coordinateRead(path, fileSystem.exists(path)), | ||
| readDirectory: (path) => | ||
| coordinateDirectoryRead(path, fileSystem.readDirectory(path)), | ||
| readFile: (path) => coordinateRead(path, fileSystem.readFile(path)), | ||
| copy: (source, destination, options) => | ||
| coordinateMutation( | ||
| [source, destination], | ||
| fileSystem.copy(source, destination, options?.overwrite) | ||
| ), | ||
| move: (source, destination) => | ||
| coordinateMutation( | ||
| [source, destination], | ||
| moveEntry(source, destination) | ||
| ), | ||
| writeFile: (path, contents) => | ||
| coordinateWrite(path, snapshotFileContents(contents)), | ||
| createFile: (path, contents) => | ||
| coordinateMutation( | ||
| [path], | ||
| createFileAt(path, snapshotFileContents(contents)) | ||
| ), | ||
| // As with unique directories, an exclusive parent lock makes candidate | ||
| // selection and creation one coordinated operation. | ||
| createUniqueFile: (parent, name, contents) => | ||
| coordinateMutation( | ||
| [parent], | ||
| createUniqueFileAt(parent, name, snapshotFileContents(contents)), | ||
| [parent] | ||
| ), | ||
| createDirectory: (path) => | ||
| coordinateMutation([path], createDirectoryAt(path)), | ||
| // Lock the parent exclusively while selecting and creating the name. | ||
| // Child mutations take a shared parent lock, so no coordinated caller | ||
| // can claim the same candidate between the existence check and creation. | ||
| createUniqueDirectory: (parent, preferredName) => | ||
| coordinateMutation( | ||
| [parent], | ||
| createUniqueDirectoryAt(parent, preferredName), | ||
| [parent] | ||
| ), | ||
| remove: (path) => coordinateMutation([path], fileSystem.remove(path)), | ||
| rename: (source, destination) => | ||
| coordinateMutation( | ||
| [source, destination], | ||
| fileSystem.rename(source, destination) | ||
| ), | ||
| }) | ||
| }) |
There was a problem hiding this comment.
This is the part I hope is most readable.
| /** | ||
| * Model the guarantees owned by one coordinator runtime. A backing adapter | ||
| * must separately guarantee that an individual successful write is atomic; | ||
| * cross-runtime ordering requires a shared platform or server authority. | ||
| */ |
There was a problem hiding this comment.
This means that we can only prove that this shit works on our current browser tab or desktop app, not that coordination occurs across clients. Although cool thing: since this uses an logical clock for versions, once a STUN server owns that we could have live collab.
| }) | ||
|
|
||
| it( | ||
| 'preserves write order and never reads an older acknowledged revision', |
There was a problem hiding this comment.
This is the big important data loss property test.
9d5a48d to
a56da93
Compare
Stacked on #13597, towards #12350, closes #13183. This adds
FileOperationsas what will become the sole application-facing filesystem authority. It uses Effect pipelines to coordinate file system operations robustly.What the
FileOperationsEffect pipeline doesstringorUint8Arraycontents for every file-writing operation, encoding strings as UTF-8 and snapshotting mutable bytes before they enter the queue.FileSystemunderneath, and adds additional client error reporting around directory and file lock failures (see "Client error reporting" below).The
FileOperationssystem is also registered as a core extension calledfileOperationsin this PR. As we complete our "strangler fig" migration it will become the only place callers are allowed to access the file system, enforced by lint rules (the next PR #13630 introduces this).Coordination guarantees
One of the core problems using Effect helps solve for us is path locking. This allows us to parallelize IO operations that are unrelated while strictly ordering ones that depend on each other: it's exactly what Effect pipelines are for. Here are some of the guarantees from the locks in this PR:
movefirst attempts rename and falls back to copy-and-remove for cross-device moves while retaining the source/destination locks for the whole compound operation.These guarantees apply to callers using this mounted
FileOperationsruntime. They do not claim atomicity against another browser tab, process, or backend writer.Client error reporting
A rejected
FileOperationsrequest emits onefile_operations_errorreport; internalFileSystemwork does not emit a second report. The report records the coordinated operation, underlying filesystem operation, typed error, and allowlisted backing error code/name.Paths, messages, stacks, filenames, and contents are excluded for user privacy. The reporter also supplies a fixed
file-operationsroute instead of inheriting the current application route, because file routes may contain encoded absolute paths.FileAlreadyExistsfrom strictcreateFileorcreateDirectoryis intentionally not reported because callers use that result for ordinary collision feedback.fast-checkis introduced in this PR for property testing / fuzzingfast-checkis specifically aimed at testing broad categories of failures by generating test inputs and expectations based on "properties". This is great for detecting and preventing race conditions, which have plagued our IO systems. They have a*.properties.*file name.The property tests in this PR generate Unicode strings, operation streams, and path sets rather than enumerating only hand-picked examples. Each generated case is checked against observable invariants, and a failure is shrunk to a smaller reproducible counterexample. Conventional tests remain alongside them as readable examples.