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.

Sign up for GitHub

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

fix(bindings): handle accessMode in runtime bindings with OPFS #1960

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+101 −42
Open

fix(bindings): handle accessMode in runtime bindings with OPFS #1960

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions packages/duckdb-wasm/src/bindings/bindings_base.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { DuckDBModule, PThread } from './duckdb_module';
import { DuckDBConfig } from './config';
import { DuckDBAccessMode, DuckDBConfig } from './config';
import { Logger } from '../log';
import { InstantiationProgress } from './progress';
import { DuckDBBindings } from './bindings_interface';
Expand Down Expand Up @@ -469,9 +469,9 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings {
}
dropResponseBuffers(this.mod);
}
public async prepareFileHandle(fileName: string, protocol: DuckDBDataProtocol): Promise<void> {
public async prepareFileHandle(fileName: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void> {
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS && this._runtime.prepareFileHandles) {
const list = await this._runtime.prepareFileHandles([fileName], DuckDBDataProtocol.BROWSER_FSACCESS);
const list = await this._runtime.prepareFileHandles([fileName], DuckDBDataProtocol.BROWSER_FSACCESS, accessMode);
for (const item of list) {
const { handle, path: filePath, fromCached } = item;
if (!fromCached && handle.getSize()) {
Expand All @@ -482,10 +482,10 @@ export abstract class DuckDBBindingsBase implements DuckDBBindings {
}
throw new Error(`prepareFileHandle: unsupported protocol ${protocol}`);
}
/** Prepare a file handle that could only be acquired aschronously */
public async prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol): Promise<void> {
/** Prepare a file handle that could only be acquired asynchronously */
public async prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void> {
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS && this._runtime.prepareDBFileHandle) {
const list = await this._runtime.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS);
const list = await this._runtime.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS, accessMode);
for (const item of list) {
const { handle, path: filePath, fromCached } = item;
if (!fromCached && handle.getSize()) {
Expand Down
6 changes: 3 additions & 3 deletions packages/duckdb-wasm/src/bindings/bindings_interface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { DuckDBConfig, DuckDBConnection, DuckDBDataProtocol, FileStatistics, InstantiationProgress } from '.';
import { DuckDBAccessMode, DuckDBConfig, DuckDBConnection, DuckDBDataProtocol, FileStatistics, InstantiationProgress } from '.';
import { CSVInsertOptions, JSONInsertOptions, ArrowInsertOptions } from './insert_options';
import { ScriptTokens } from './tokens';
import { WebFile } from './web_file';
Expand Down Expand Up @@ -54,8 +54,8 @@ export interface DuckDBBindings {
protocol: DuckDBDataProtocol,
directIO: boolean,
): Promise<HandleType>;
prepareFileHandle(path: string, protocol: DuckDBDataProtocol): Promise<void>;
prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol): Promise<void>;
prepareFileHandle(path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void>;
prepareDBFileHandle(path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<void>;
globFiles(path: string): WebFile[];
dropFile(name: string): void;
dropFiles(): void;
Expand Down
11 changes: 7 additions & 4 deletions packages/duckdb-wasm/src/bindings/runtime.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { DuckDBAccessMode } from './config';
import { DuckDBModule } from './duckdb_module';
import { UDFFunction } from './udf_function';
import * as udf_rt from './udf_runtime';

export { DuckDBAccessMode };

/** Wrapper for TextDecoder to support shared array buffers */
function TextDecoderWrapper(): (input?: BufferSource) => string {
const decoder = new TextDecoder();
Expand Down Expand Up @@ -156,10 +159,10 @@ export interface DuckDBRuntime {
checkFile(mod: DuckDBModule, pathPtr: number, pathLen: number): boolean;
removeFile(mod: DuckDBModule, pathPtr: number, pathLen: number): void;

// Prepare a file handle that could only be acquired aschronously
prepareFileHandle?: (path: string, protocol: DuckDBDataProtocol) => Promise<PreparedDBFileHandle[]>;
prepareFileHandles?: (path: string[], protocol: DuckDBDataProtocol) => Promise<PreparedDBFileHandle[]>;
prepareDBFileHandle?: (path: string, protocol: DuckDBDataProtocol) => Promise<PreparedDBFileHandle[]>;
// Prepare a file handle that could only be acquired asynchronously
prepareFileHandle?: (path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode) => Promise<PreparedDBFileHandle[]>;
prepareFileHandles?: (path: string[], protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode) => Promise<PreparedDBFileHandle[]>;
prepareDBFileHandle?: (path: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode) => Promise<PreparedDBFileHandle[]>;

// Call a scalar UDF function
callScalarUDF(
Expand Down
30 changes: 20 additions & 10 deletions packages/duckdb-wasm/src/bindings/runtime_browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {addS3Headers, getHTTPUrl} from '../utils';
import {
callSRet,
dropResponseBuffers,
DuckDBAccessMode,
DuckDBDataProtocol,
DuckDBFileInfo,
DuckDBGlobalFileInfo,
Expand Down Expand Up @@ -110,8 +111,11 @@ export const BROWSER_RUNTIME: DuckDBRuntime & {
BROWSER_RUNTIME._opfsRoot = await navigator.storage.getDirectory();
}
},
/** Prepare a file handle that could only be acquired aschronously */
async prepareFileHandles(filePaths: string[], protocol: DuckDBDataProtocol): Promise<PreparedDBFileHandle[]> {
/** Prepare a file handle that could only be acquired asynchronously */
async prepareFileHandles(filePaths: string[], protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<PreparedDBFileHandle[]> {
// DuckDBAccessMode.UNDEFINED will be treated as READ_WRITE
// See: https://github.com/duckdb/duckdb/blob/5f5512b827df6397afd31daedb4bbdee76520019/src/main/database.cpp#L442-L444
const isReadOnly = accessMode !== undefined && accessMode === DuckDBAccessMode.READ_ONLY;
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS) {
await BROWSER_RUNTIME.assignOPFSRoot();
const prepare = async (path: string): Promise<PreparedDBFileHandle> => {
Expand All @@ -133,15 +137,21 @@ export const BROWSER_RUNTIME: DuckDBRuntime & {
if (!fileName) {
throw new Error(`Invalid path ${path}`);
}
// mkdir -p
for (const folder of folders) {
dirHandle = await dirHandle.getDirectoryHandle(folder, { create: true });
// Check read-only
if (!isReadOnly) {
// mkdir -p
for (const folder of folders) {
dirHandle = await dirHandle.getDirectoryHandle(folder, { create: true });
}
}
}
const fileHandle = await dirHandle.getFileHandle(fileName, { create: false }).catch(e => {
if (e?.name === 'NotFoundError') {
console.debug(`File ${path} does not exists yet, creating...`);
return dirHandle.getFileHandle(fileName, { create: true });
if (!isReadOnly) {
console.debug(`File ${path} does not exists yet, creating...`);
return dirHandle.getFileHandle(fileName, { create: true });
}
console.debug(`File ${path} does not exists, aborting as we are in read-only mode`);
Copy link

Choose a reason for hiding this comment

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

Five lines below this, where we call createSyncAccessHandle, we can pass a new optional mode to OPFS to further enforce the access mode: https://developer.mozilla.org/en-US/docs/Web/API/FileSystemFileHandle/createSyncAccessHandle#mode

If the access mode is passed to createSyncAccessHandle, it would allow read-only connections to be opened simultaneously (for example when using the same app in multiple tabs). A big win for duckdb-wasm + OPFS.

The option is only implemented in Chrome and Edge right now, but issues exist to implement in both Webkit and Firefox:
https://bugs.webkit.org/show_bug.cgi?id=283959
https://bugzilla.mozilla.org/show_bug.cgi?id=1949462

And tests have already been added to the Web Platform Tests dashboard:
https://wpt.fyi/results/fs/FileSystemFileHandle-sync-access-handle-lock-modes.https.tentative.worker.html?label=experimental&label=master&aligned

Copy link

Choose a reason for hiding this comment

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

Ah, just noticed you started a new PR for this! Will comment over there instead.

}
throw e;
});
Expand All @@ -166,11 +176,11 @@ export const BROWSER_RUNTIME: DuckDBRuntime & {
}
throw new Error(`Unsupported protocol ${protocol} for paths ${filePaths} with protocol ${protocol}`);
},
/** Prepare a file handle that could only be acquired aschronously */
async prepareDBFileHandle(dbPath: string, protocol: DuckDBDataProtocol): Promise<PreparedDBFileHandle[]> {
/** Prepare a file handle that could only be acquired asynchronously */
async prepareDBFileHandle(dbPath: string, protocol: DuckDBDataProtocol, accessMode?: DuckDBAccessMode): Promise<PreparedDBFileHandle[]> {
if (protocol === DuckDBDataProtocol.BROWSER_FSACCESS && this.prepareFileHandles) {
const filePaths = [dbPath, `${dbPath}.wal`];
return this.prepareFileHandles(filePaths, protocol);
return this.prepareFileHandles(filePaths, protocol, accessMode);
}
throw new Error(`Unsupported protocol ${protocol} for path ${dbPath} with protocol ${protocol}`);
},
Expand Down
3 changes: 2 additions & 1 deletion packages/duckdb-wasm/src/parallel/worker_dispatcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,9 @@ export abstract class AsyncDuckDBDispatcher implements Logger {

case WorkerRequestType.OPEN: {
const path = request.data.path;
const accessMode = request.data.accessMode;
if (path?.startsWith('opfs://')) {
await this._bindings.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS);
await this._bindings.prepareDBFileHandle(path, DuckDBDataProtocol.BROWSER_FSACCESS, accessMode);
request.data.useDirectIO = true;
}
this._bindings.open(request.data);
Expand Down
81 changes: 63 additions & 18 deletions packages/duckdb-wasm/test/opfs.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,30 +278,75 @@ export function testOPFS(baseDir: string, bundle: () => duckdb.DuckDBBundle): vo
});
});

async function removeFiles() {
const opfsRoot = await navigator.storage.getDirectory();
await opfsRoot.removeEntry('test.db').catch(() => {
});
await opfsRoot.removeEntry('test.db.wal').catch(() => {
});
await opfsRoot.removeEntry('test.csv').catch(() => {
});
await opfsRoot.removeEntry('test1.csv').catch(() => {
});
await opfsRoot.removeEntry('test2.csv').catch(() => {
});
await opfsRoot.removeEntry('test3.csv').catch(() => {
describe('Open database in OPFS', () => {
it('should not open a non-existent DB file in read-only', async () => {
const logger = new duckdb.ConsoleLogger(LogLevel.ERROR);
const worker = new Worker(bundle().mainWorker!);
const db_ = new duckdb.AsyncDuckDB(logger, worker);
await db_.instantiate(bundle().mainModule, bundle().pthreadWorker);

await expectAsync(db_.open({
path: 'opfs://non_existent.db',
accessMode: duckdb.DuckDBAccessMode.READ_ONLY,
})).toBeRejectedWithError(Error, /file or directory could not be found/);

await db_.terminate();
await worker.terminate();

// Files should not be found with DuckDBAccessMode.READ_ONLY
const opfsRoot = await navigator.storage.getDirectory();
await expectAsync(opfsRoot.getFileHandle('non_existent.db', { create: false }))
.toBeRejectedWithError(Error, /file or directory could not be found/);
});
await opfsRoot.removeEntry('test.parquet').catch(() => {

it('should open a non-existent DB file in read-write and create files', async () => {
const logger = new duckdb.ConsoleLogger(LogLevel.ERROR);
const worker = new Worker(bundle().mainWorker!);
const db_ = new duckdb.AsyncDuckDB(logger, worker);
await db_.instantiate(bundle().mainModule, bundle().pthreadWorker);

const opfsRoot = await navigator.storage.getDirectory();

// Ensure files do not exist
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db', { create: false }))
.toBeRejectedWithError(Error, /file or directory could not be found/);
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db.wal', { create: false }))
.toBeRejectedWithError(Error, /file or directory could not be found/);

await expectAsync(db_.open({
path: 'opfs://non_existent_2.db',
accessMode: duckdb.DuckDBAccessMode.READ_WRITE,
})).toBeResolved();

await db_.terminate();
await worker.terminate();

// Files should be found with DuckDBAccessMode.READ_WRITE
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db', { create: false })).toBeResolved();
await expectAsync(opfsRoot.getFileHandle('non_existent_2.db.wal', { create: false })).toBeResolved();
});
})

async function removeFiles() {
const opfsRoot = await navigator.storage.getDirectory();
await opfsRoot.removeEntry('test.db').catch(() => {});
await opfsRoot.removeEntry('test.db.wal').catch(() => {});
await opfsRoot.removeEntry('test.csv').catch(() => {});
await opfsRoot.removeEntry('test1.csv').catch(() => {});
await opfsRoot.removeEntry('test2.csv').catch(() => {});
await opfsRoot.removeEntry('test3.csv').catch(() => {});
await opfsRoot.removeEntry('test.parquet').catch(() => {});
try {
const datadir = await opfsRoot.getDirectoryHandle('datadir');
datadir.removeEntry('test.parquet').catch(() => {
});
datadir.removeEntry('test.parquet').catch(() => {});
} catch (e) {
//
}
await opfsRoot.removeEntry('datadir').catch(() => {
});
await opfsRoot.removeEntry('datadir').catch(() => {});
// In case of failure caused leftovers
await opfsRoot.removeEntry('non_existent.db').catch(() => {});
await opfsRoot.removeEntry('non_existent.db.wal').catch(() => {});
await opfsRoot.removeEntry('non_existent_2.db').catch(() => {});
await opfsRoot.removeEntry('non_existent_2.db.wal').catch(() => {});
}
}
Loading