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

Add lock to indexeddb vfs so that the database can only be opened once. #85

Open
Neo-Ciber94 opened this issue Apr 30, 2024 · 11 comments
Open

Comments

@Neo-Ciber94
Copy link

Neo-Ciber94 commented Apr 30, 2024

Each time I try to use the IndexedDb I get this error.

this.program: could not access the server configuration file "/pgdata/postgresql.conf": No such file or directory

image

This is the code that trigger it:

"use client";

import { PGlite } from "@electric-sql/pglite";
import { useEffect, useState } from "react";

export default function PgLiteComponent() {
  const [data, setData] = useState<any>("PENDING");

  useEffect(() => {
    (async () => {
      const client = new PGlite("idb://local", { debug: 1 });
      const result = await client.exec("SELECT NOW()");
      setData(result);
    })();
  }, []);

  return (
    <div>
      <h1>PGLite</h1>
      <pre>{JSON.stringify(data ?? "", null, 2)}</pre>
    </div>
  );
}

The app halts, the error cannot be caught, so is not possible to do anything.

@samwillis
Copy link
Collaborator

Thanks for the report, could you let me know what React framework and build tooling you are using?

the "use client" is making me think you may be using Next.js and RSC?

Does this happen in all web browsers or only a specific one?

@Neo-Ciber94
Copy link
Author

Neo-Ciber94 commented Apr 30, 2024

Thanks for the report, could you let me know what React framework and build tooling you are using?

the "use client" is making me think you may be using Next.js and RSC?

Does this happen in all web browsers or only a specific one?

I test the code on NextJS, RemixJS and Vite+React, all 3 the same error.

I was using Chrome (Windows).

@samwillis
Copy link
Collaborator

Could you check if there is an indexeddb via the Chrome dev tools, if there is one delete it and refresh the app. It may be that the state of the database is broken, I'm suspicious that the initdb phase didn't complete when you first ran the code. Alternatively change the name of the database so that it creates a new one.

@Neo-Ciber94
Copy link
Author

Removing or changing the name of the Indexeddb is not working for me either, is also happening in Firefox:

image

@Neo-Ciber94
Copy link
Author

Neo-Ciber94 commented Apr 30, 2024

Not sure where the errors is comming from, is also happening on stackblitz:

https://stackblitz.com/edit/react-pglite-bug-rcynzw?file=src%2FApp.tsx

Be aware that this halts my browser.

image

@samwillis
Copy link
Collaborator

Hey @Neo-Ciber94

I've found the problem, it's due to React strict mode. Essentially PGlite is initiated twice for the same database, the first time it creates the indexeddb database to store the files, the second time it sees the database is there and so thinks it doesn't have to initiate it. However the first run hasn't completed and so the database doesn't exist.

I commented out the strict mode on this fork: https://stackblitz.com/edit/react-pglite-bug-rcynzw-cwupa2?file=src%2FApp.tsx

It's one of the occasions where you almost certainly don't want strict mode to init the postgres twice as it's such a cpu heavy operation, even for development. I would recommend explicitly only having a single instance of PGlite, example here (and below): https://stackblitz.com/edit/react-pglite-bug-rcynzw-bzsd5x?file=src%2FApp.tsx,src%2Fmain.tsx,src%2Findex.css

We should look at having a lock so that only one instance of the database can be opened at once with indexeddb and throw an error if it can't be acquired. I'm going to use this issue to track that.

import { PGlite } from '@electric-sql/pglite';
import { useState, useEffect } from 'react';

const DATA_DIR = 'idb://somedatabase2';

let client: PGlite;

export default function App() {
  const [data, setData] = useState<unknown>('PENDING');

  useEffect(() => {
    (async () => {
      client ??= new PGlite(DATA_DIR, { debug: 1 });
      const result = await client.exec('SELECT NOW()');
      setData(result);
    })();
  }, []);

  return (
    <div>
      <h1>PGLite</h1>
      <pre>{JSON.stringify(data, null, 2)}</pre>
    </div>
  );
}

@samwillis samwillis changed the title this.program: could not access the server configuration file "/pgdata/postgresql.conf": No such file or directory Add lock to indexeddb vfs so that the database can only be opened once. Apr 30, 2024
@Neo-Ciber94
Copy link
Author

@samwillis You are totally right, looks like useEffect took other victim.

Ensuring the instance is created only once solve the issue for me.

I really appreciate your help, Thank you!

@thruflo
Copy link
Contributor

thruflo commented May 1, 2024

Perhaps we could provide a PGlite.createOnce() function or similar, which would return the same instance no matter how many times it’s called.

@samwillis
Copy link
Collaborator

@thruflo i like that. Ideally we should use a weak map to store the references to previously open databases so they can be cleanly garbage collected without having to explicitly close it to remove any reference.

@berzi
Copy link

berzi commented Jun 2, 2024

If a createOnce() is added, would there be any use for the current syntax that allows multiple instances to be created?

Also is there anything to consider to support workers? Maybe we need a createOnceWithWorkers() too? Even though I don't really know why you would ever need an instance on the main thread in the first place—unless there's a good reason it sounds like an antipattern to even allow it.

@davidng10
Copy link

Hey @Neo-Ciber94

I've found the problem, it's due to React strict mode. Essentially PGlite is initiated twice for the same database, the first time it creates the indexeddb database to store the files, the second time it sees the database is there and so thinks it doesn't have to initiate it. However the first run hasn't completed and so the database doesn't exist.

I commented out the strict mode on this fork: https://stackblitz.com/edit/react-pglite-bug-rcynzw-cwupa2?file=src%2FApp.tsx

It's one of the occasions where you almost certainly don't want strict mode to init the postgres twice as it's such a cpu heavy operation, even for development. I would recommend explicitly only having a single instance of PGlite, example here (and below): https://stackblitz.com/edit/react-pglite-bug-rcynzw-bzsd5x?file=src%2FApp.tsx,src%2Fmain.tsx,src%2Findex.css

We should look at having a lock so that only one instance of the database can be opened at once with indexeddb and throw an error if it can't be acquired. I'm going to use this issue to track that.

import { PGlite } from '@electric-sql/pglite';
import { useState, useEffect } from 'react';

const DATA_DIR = 'idb://somedatabase2';

let client: PGlite;

export default function App() {
  const [data, setData] = useState<unknown>('PENDING');

  useEffect(() => {
    (async () => {
      client ??= new PGlite(DATA_DIR, { debug: 1 });
      const result = await client.exec('SELECT NOW()');
      setData(result);
    })();
  }, []);

  return (
    <div>
      <h1>PGLite</h1>
      <pre>{JSON.stringify(data, null, 2)}</pre>
    </div>
  );
}

Just a thought within the context of React, have we considered initiating the PG instance outside of the react component? With React 18, we can add a Suspense as well if you wish to have a loading state. Disabling StrictMode simply to ensure PG instance is initiated once, to me, feels like going against the very reason why StrictMode was implemented in the first place.

// RootLayout.tsx
const db = await PGlite.create({
  extensions: { live }
});

export default function RootLayout({
  children,
}: {
  children: React.ReactNode;
}) {
  return (
    <>
      <PGliteProvider db={db}>{children}</PGliteProvider>
    </>
  );
}

// main.tsx
const RootLayout = lazy(() => import("../components/RootLayout"));
export function Main(){
    return (
        <Suspense fallback={<div>Loading...</div>}>
            <RootLayout />
        </Suspense>
    )
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants