SanderGi / sqlite-auto-migrator

Simple, flexible and automated SQLite database migration tool
https://www.npmjs.com/package/sqlite-auto-migrator
MIT License
0 stars 1 forks source link

Randomly Occurring Deadlock #2

Closed SanderGi closed 1 hour ago

SanderGi commented 2 weeks ago

I have issues with it getting stuck and never finishing. It only happens randomly though.

image

image

When that happens, the tables are left in a locked state:

image

Originally posted by @buzzy in https://github.com/SanderGi/sqlite-auto-migrator/issues/1#issuecomment-2372377631

SanderGi commented 2 weeks ago

@buzzy, can you confirm how you are running this (presumably the same code as #1)? The first screenshot has "Diffing schema" twice which indicates migrator.make() was called twice so just want to confirm that you are not accidentally calling it twice concurrently (that could definitely cause a deadlock since the library is not meant for multiple concurrent processes)

buzzy commented 2 weeks ago

I am running it once for each database that I have.

import { Migrator } from "sqlite-auto-migrator";
import { config } from "../config/config";

// @ts-ignore
import sessions from "../databases/sessions.sql";
// @ts-ignore
import main from "../databases/main.sql";

export async function updateDatabase() {
  // Exit early if --update-database flag is not present
  if (Bun.argv.indexOf("--update-database") === -1) return;

  try {
    // Session database
    const migrator = new Migrator({
      dbPath: config.auth.database,
      schemaPath: sessions,
      createDBIfMissing: true,
    });

    await migrator.make({
      onRename: Migrator.REQUIRE_MANUAL_MIGRATION,
      onDestructiveChange: Migrator.REQUIRE_MANUAL_MIGRATION,
      onChangedView: Migrator.REQUIRE_MANUAL_MIGRATION,
      onChangedIndex: Migrator.PROCEED,
      onChangedTrigger: Migrator.PROCEED,
      createIfNoChanges: false,
    });

    await migrator.migrate();

    // Main database
    const migrator2 = new Migrator({
      dbPath: config.database,
      schemaPath: main,
      createDBIfMissing: true,
    });

    await migrator2.make({
      onRename: Migrator.REQUIRE_MANUAL_MIGRATION,
      onDestructiveChange: Migrator.REQUIRE_MANUAL_MIGRATION,
      onChangedView: Migrator.REQUIRE_MANUAL_MIGRATION,
      onChangedIndex: Migrator.PROCEED,
      onChangedTrigger: Migrator.PROCEED,
      createIfNoChanges: false,
    });

    await migrator2.migrate();
  } catch (e) {
    console.error(e);
    process.exit(1);
  }

  console.log("Databases updated successfully!");

  process.exit(0);
}

I am using simple test schemas:

-- Table: users
CREATE TABLE users (id BLOB PRIMARY KEY, mail TEXT, password TEXT) WITHOUT ROWID;

-- Index: mail
CREATE INDEX mail ON users (mail);

Second one:

-- Table: sessions
CREATE TABLE "sessions" (
  id blob PRIMARY KEY NOT NULL,
  user blob NOT NULL,
  lastChecked timestamp NOT NULL
) WITHOUT ROWID;

-- Index: user
CREATE INDEX user ON sessions (user);
buzzy commented 1 week ago

Btw, the lock is often happening when there is no database, thus when sqlite-auto-migration needs to create the database from scratch.

SanderGi commented 1 week ago

Thank you for the detailed code. I'm able to reproduce the intermittent deadlock and looking into a fix.

One thing to note with your code, you are creating migrators for two different databases but keeping the default migrationPath parameter meaning they share the same 'migrations' directory. This won't be an issue with declarative diffing (#3) but your code as is would mix migration state between the databases and likely lead to invalid migrations. I'll add a note in the documentation about this pitfall and have the library display a warning when creating multiple instances of the migrator with the same migrations directory

SanderGi commented 5 days ago

Using the built-in bun:sqlite (#1) seems to get rid of the deadlocks for me: I ran the above code in a loop 100 times with deletion of the database files inbetween iterations and it did not get stuck at all. Can you confirm if this is the case for you too? If so, perhaps the error is with my promise based adapter for node-sqlite3 🤔

SanderGi commented 1 hour ago

Version 1.2.0 has been published to NPM and includes the deadlock fix