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

Don't use temp file when restoring to in-memory SQLite #739

Merged
merged 16 commits into from
Feb 1, 2021

Conversation

otoolep
Copy link
Member

@otoolep otoolep commented Jan 29, 2021

Saves a read and write, the size of the SQLite database, when restoring an in-memory database.

This saves a read and write to disk, the size of the SQLite database.
@otoolep otoolep changed the title No disk during restore Don't use temp file when restoring to in-memory SQLite Jan 29, 2021
@otoolep
Copy link
Member Author

otoolep commented Jan 29, 2021

When I actually run a node, have it do a few snapshots, and then restart it -- the CLI is getting "not a database".

@otoolep
Copy link
Member Author

otoolep commented Jan 29, 2021

Don't merge this, not until I understand the errors (and if they are real, why the test suite isn't catching them).

@otoolep
Copy link
Member Author

otoolep commented Jan 29, 2021

Seems fine, can't reproduce the issue via the CLI. I've also added end-to-end testing to ensure restoring to both in-mem and on-disk databases works fine.

This change also adds better logging, so the restoration process can be
better observed.
@otoolep
Copy link
Member Author

otoolep commented Jan 29, 2021

I got the system into this state again.

127.0.0.1:4001> select count(*) from foo
ERR! file is not a database
127.0.0.1:4001> select count(*) from foo
ERR! file is not a database
127.0.0.1:4001> 
EOF (CTRL+D)
~/repos/rqlite/src/github.com/rqlite/rqlite/cmd/rqlite (no-disk-during-restore)$ ./rqlite 
Welcome to the rqlite CLI. Enter ".help" for usage hints.
Connected to version 5
127.0.0.1:4001> select count(*) from foo
ERR! file is not a database
127.0.0.1:4001> select count(*) from foo
ERR! file is not a database
127.0.0.1:4001> select count(*) from foo
ERR! file is not a database
127.0.0.1:4001> 

Not good.

@otoolep
Copy link
Member Author

otoolep commented Jan 29, 2021

The test case seems to be:

  • start a node using in-mem database
  • trigger snapshot
  • restart node
  • queries break, with "not a database".

@otoolep
Copy link
Member Author

otoolep commented Jan 29, 2021

Very strange that none of the current testing catches this, only the command line testing.

@otoolep
Copy link
Member Author

otoolep commented Jan 31, 2021

I think I see the issue. This problem doesn't occur 100% of the time with the following script, but pretty close:

#!/bin/bash
DATA_DIR=`mktemp -d`

$RQLITED -raft-snap 13 -raft-snap-int=1s $DATA_DIR &
sleep 3

$RQBENCH -b 10 -m 1 -n 500 -o 'CREATE TABLE foo (id INTEGER NOT NULL PRIMARY KEY, name TEXT)' 'INSERT INTO foo(name) VALUES("fiona")'

killall rqlited
sleep 3

$RQLITED -raft-snap 50000 -raft-snap-int=500s $DATA_DIR

SQLite is using the memory passed in, and making changes to it -- that memory represents the database after all. But that memory is in the Go memory space. And I'm sure this is allowed, since Go is a garbage collected language.

@otoolep otoolep merged commit 1c9fa88 into master Feb 1, 2021
@otoolep otoolep deleted the no-disk-during-restore branch February 1, 2021 01:55
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

Successfully merging this pull request may close these issues.

None yet

1 participant