vegarringdal / sqllitedebug

1 stars 0 forks source link

sqllite many workers #1

Open vegarringdal opened 1 year ago

vegarringdal commented 1 year ago

Hi @sgbeal

Been trying to do a lot of weird stuff in this repo.

if I remove these its actually running very long without failing

I dont know if this helps

image

image

sgbeal commented 1 year ago

That is definitely very interesting but i'm at a loss to speculate about what causes it :(. i cannot say for sure when i'll be able to actively pursue this. i'll be living on a boat through at least July, maybe August, while hunting for a new residence, with only an old laptop to work from when i'm on land (where there's AC electricity), so my investigation will be slow for the foreseeable future :/.

In any case, any details like these you find are liable to be helpful in finding the culprit.

sgbeal commented 1 year ago

Just FYI, i'm now following this repo so will get notifications for all changes.

vegarringdal commented 1 year ago

i'll be living on a boat through at least July, maybe August, while hunting for a new residence, with only an old laptop to work from when i'm on land (where there's AC electricity), so my investigation will be slow for the foreseeable future :/.

NP, Im just glad someone else trying and look into it when they have time. Thats why Im trying to reproduce errors 😄 My current workaround in the app Im making is I do all large delete/create in sequence over my workers.

So the issue is really when there is a large/long running update from what I can tell. Since when i do queries in parallel I never get any issues. Maybe some file locks/timeouts triggering when there should not be a timeout.. Tried to look into some of the js code... but there is alot 😂

Good luck on your residence hunting btw 👍

vegarringdal commented 1 year ago

@sgbeal

I can prob find out this myself, but might be easier to ask. 😄

The sharedArrayBuffers, is sqllite using this due to communicate with async helper worker only, or is it needed for communication with the wasm part? Have a app I would like to use OPFS and sqllite, but need cross origin access with post message on window.top/parent on pages I open to send events to a page.

sgbeal commented 1 year ago

It's only used for OPFS communication. SQLite has all synchronous APIs and OPFS is largely async. The only mechanism available for/known by us to coordinate that communication between two threads, without resorting to 3rd-party tools like Asyncify, is the SAB and Atomics APIs.

vegarringdal commented 1 year ago

So in theory its possible to remove SAB with the current syncHandles OPFS have gotten when you are in a worker? Only getting/opening the db that would be async. Or maybe there is something Im overlooking.

Thanks for taking the time for my weird questions btw 👍

sgbeal commented 1 year ago

So in theory its possible to remove SAB with the current syncHandles OPFS have gotten when you are in a worker?

Nope. So long as OPFS has even one async method, it has to run in a separate thread from the C-level code, and the SAB/Atomics combo is needed to coordinate the comms between those.

Only getting/opening the db that would be async. Or maybe there is something Im overlooking.

The OPFS handle is only accessible from that thread and that thread is necessarily a different one from where SQLite is running. We cannot synchronously open an OPFS handle from the C-level API because of how the async/await mechanism in JS works.

Sidebar: OPFS being even partially async is a design flaw, IMO. Async APIs exist to account for high latency, whereas OPFS has none. This was a big topic in OPFS's design and in my meetings with the Chrome folks about the difficulties in bridging async and sync APIs in JS, but the Async All The Things camp won that battle. Integrating OPFS with WASM would be child's play if it were 100% synchronous, but so long as even a single method is async, that is "viral" and forces us to run the OPFS parts in a separate thread from the main/synchronous parts.

vegarringdal commented 1 year ago

Ok

So much I do not know about this topic 😂 I thought we could stay in a worker and have a async open method to get db context only. But maybe more methods is async/inner workings above my head. 😄 Was hoping the async method was mainly because there was no synchandle in the first releases.

Do you know if they plan to add more sync methods for worker context?


async function open() {
  const root = await navigator.storage.getDirectory();
  const draftHandle = await root.getFileHandle("mydb.db", { create: true });
  const accessHandle = await draftHandle.createSyncAccessHandle();
  // give handle to wasm magic part ;-)
  // return db context to user.
}
sgbeal commented 1 year ago

Do you know if they plan to add more sync methods for worker context?

No idea - i don't actively follow future developments. So long as even a single method is async, though, it will continue to be painful to integrate with any purely synchronous code (like sqlite's C API).

Your open() example would ideally be a valid approach, but it doesn't work that nicely because obtaining and releasing sync access handles is async and something we have to do frequently to enable concurrency. OPFS exclusively locks a file when a sync access handle is acquired, so inherently supports no concurrency whatsoever. We have to obtain/release those handles to squeeze any concurrency out of it.

Additionally, even closing a handle is async but sqlite requires it to be synchronous because it often deletes temp files immediately after closing them. Flagging a JS function ad "async" implicitly changes its return semantics to something C cannot work with, so async functions cannot be combined with C-level code like the sqlite VFS requires.

vegarringdal commented 1 year ago

Sorry for all the questions, really appreciate you taking the time to answer.

I never really thought about temp files/journal files it needs to create. Temp files/journal would prob create issue on my weird ideas about working around the cross origin issue, and prob not easy to work around by building a queue to stop concurrency.

The close method is sync btw from what i understand, so we can hope it gets better in time. Just need 3 more methods as sync then 😄

https://developer.mozilla.org/en-US/docs/Web/API/FileSystemSyncAccessHandle

onmessage = async (e) => {
  // Retrieve message sent to work from main script
  const message = e.data;

  // Get handle to draft file
  const root = await navigator.storage.getDirectory();
  const draftHandle = await root.getFileHandle("draft.txt", { create: true });
  // Get sync access handle
  const accessHandle = await draftHandle.createSyncAccessHandle();

  // Get size of the file.
  const fileSize = accessHandle.getSize();
  // Read file content to a buffer.
  const buffer = new DataView(new ArrayBuffer(fileSize));
  const readBuffer = accessHandle.read(buffer, { at: 0 });

  // Write the message to the end of the file.
  const encoder = new TextEncoder();
  const encodedMessage = encoder.encode(message);
  const writeBuffer = accessHandle.write(encodedMessage, { at: readBuffer });

  // Persist changes to disk.
  accessHandle.flush();

  // Always close FileSystemSyncAccessHandle if done.
  accessHandle.close();
};
sgbeal commented 1 year ago

close() might be sync now but we are stuck supporting versions where it's not, so it doesn't get us to the 100% sync API we would need in order to remove the SAB. The one async func we could work around is the initial getDirecory() call. It would be possible to delay init of the library until that returns but we still can't open files/sync handles synchronously, so require the SAB spaghetti.

VegarRingdalAibel commented 1 year ago

Note to my self, have a look if this fixes my issues https://sqlite.org/src/info/c9fdd680