Giter VIP home page Giter VIP logo

Comments (12)

tsibley avatar tsibley commented on July 19, 2024 1

Testing with Datasette using multiple threads is likely to give misleading results, as your different requests may hit different threads which only open the SQLite file as necessary. This is why different threads in the lsof output above had different inodes opened.

Using cp instead is a good thought! But I'm wary of overwriting the entire SQLite file in-place, as that may modify other parts of the SQLite file structure behind Datasette's back and lead to other subtle issues. Depending on how the SQLite libraries handle this, it might be ok, but I don't know SQLite's guarantees here.

One option might be to leave the SQLite file itself alone and instead do the replacement atomically within SQL. That is, within a single transaction, truncate the data table before insert. I added support to sqlite-utils for this yesterday in a PR, thinking we might need it.

from switchboard.

kairstenfay avatar kairstenfay commented on July 19, 2024

From @tsibley:

My guess is that this issue is related to how we're updating the SQLite file. Particularly if Datasette holds open file handles to it.

If we can replicate locally, then using lsof to look at the inodes the Datasette processes have open and comparing to the inode of the latest SQLite file would be helpful next time this occurs.

from switchboard.

tsibley avatar tsibley commented on July 19, 2024

Reproducing locally might also be easier by running Datasette single-threaded so that test requests are sure to hit the same process/thread.

In this code:

https://github.com/seattleflu/scan-switchboard/blob/92a9a9ecbbe9e035f51821bad60fd611d79e1ade/Makefile#L6-L8

We're not updating the SQLite file in place, but replacing it with a different file. That was intentional to avoid a bad update leaving an existing but broken database file, but it may not place nice with Datasette. Casual testing during dev seemed to show that Datasette/SQLite copes just fine, but more rigorous testing may show otherwise.

from switchboard.

tsibley avatar tsibley commented on July 19, 2024

Fortuitously, this just happened again in production. I grabbed an lsof snapshot of the Datasette process (PID reported by systemctl status scan-switchboard) and stat'd the current SQLite file.

image

The current SQLite file is inode 2560088, and as suspected, the Datasette threads are still holding open file descriptors for the deleted files. lsof helpfully reports this, but you can also see the inodes (last number before the file path) are different (2560067 and 2560061). They're not even consistent across threads, since the threads presumably opened the path at different times.

I think in-place updates will work if we stop re-creating/overwriting the database file everytime but instead only update it in-place via SQL commands. But we should robustly test this locally, and to do so consistently will need to make sure Datasette is running single-threaded. (Not sure if that's possible, though?)

from switchboard.

joverlee521 avatar joverlee521 commented on July 19, 2024

Can we just update in-place via cp?

from switchboard.

kairstenfay avatar kairstenfay commented on July 19, 2024

Working on this now, but wanted to say that reverting commit de07e5c4 produces the same error (of datasette pointing to a deleted sqlite file).

Same goes for @joverlee521's cp/rm suggestion, unfortunately.

from switchboard.

kairstenfay avatar kairstenfay commented on July 19, 2024

If I understand @tsibley correctly, updating the sqlite file in place also results in the same error.

Never mind, further testing seems to show it does fine.

data/scan-redcap.sqlite: data/record-barcodes.ndjson derived-tables.sql
	sqlite-utils insert --nl $@ record_barcodes $<
	sqlite3 $@ < derived-tables.sql

from switchboard.

kairstenfay avatar kairstenfay commented on July 19, 2024

Ah, my original testing process was flawed. It appears both @joverlee521's solution,

data/scan-redcap.sqlite: data/record-barcodes.ndjson derived-tables.sql
	sqlite-utils insert --nl [email protected] record_barcodes $<
	sqlite3 [email protected] < derived-tables.sql
	cp [email protected] $@
	rm [email protected]

, and updating in place (my code in the previous comment) avoid Datasette threads' holding open file descriptors of deleted files.
Although, the in place solution does not remove deleted barcodes.

from switchboard.

kairstenfay avatar kairstenfay commented on July 19, 2024

Description of my local testing:

  1. restart the service with systemctl restart scan-switchboard
  2. run make -B but with a project, e.g. SCAN IRB English, removed.
  3. run make -B again but including the project removed in step 2.
  4. see if the web app can access the newly included data

from switchboard.

tsibley avatar tsibley commented on July 19, 2024

The num_sql_threads option to Datasette allows specifying how many Python threads are used to open the SQLite file and run queries. If I set it to 1 by:

pipenv run ./bin/serve --config num_sql_threads:1

then overwrite the file in-place with cp and make a new request, I see this error in the logs:

ERROR: conn=<sqlite3.Connection object at 0x7f27e1bfd1f0>, sql = 'select count(*) from [duplicate_record_ids]', params = None: database disk image is malformed

from switchboard.

kairstenfay avatar kairstenfay commented on July 19, 2024

The num_sql_threads option to Datasette allows specifying how many Python threads are used to open the SQLite file and run queries. If I set it to 1 by:

pipenv run ./bin/serve --config num_sql_threads:1

then overwrite the file in-place with cp and make a new request, I see this error in the logs:

ERROR: conn=<sqlite3.Connection object at 0x7f27e1bfd1f0>, sql = 'select count(*) from [duplicate_record_ids]', params = None: database disk image is malformed

Hey Tom, where do you see this error? I invoked ./bin/serve with the num_sql_threads option, and I don't see that error logged in my syslog, systemctl or journalctl status outputs.

from switchboard.

tsibley avatar tsibley commented on July 19, 2024

It's in the terminal output when I run

pipenv run ./bin/serve --config num_sql_threads:1

from switchboard.

Related Issues (4)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.