performance: add a verification file to the database (#14693)

Reading the database repeatedly can be quite slow.  We need a way to speed
up Spack when it reads the DB multiple times, but the DB has not been
modified between reads (which is nearly all the time).

- [x] Add a file containing a unique uuid that is regenerated at database
    write time. Use this uuid to suppress re-parsing the database
    contents if we know a previous uuid and the uuid has not changed.

- [x] Fix mutable_database fixture so that it resets the last seen
    verifier when it resets.

- [x] Enable not rereading the database immediately after a write. Make
    the tests reset the last seen verifier in between tests that use the
    database fixture.

- [x] make presence of uuid module optional
This commit is contained in:
Andrew W Elble 2020-03-20 20:05:51 -04:00 committed by Todd Gamblin
parent 9b5805a5cd
commit 6b559912c1
3 changed files with 52 additions and 3 deletions

View file

@ -26,6 +26,12 @@
import socket
import sys
import time
try:
import uuid
_use_uuid = True
except ImportError:
_use_uuid = False
pass
import llnl.util.tty as tty
import six
@ -294,6 +300,7 @@ def __init__(self, root, db_dir=None, upstream_dbs=None,
# Set up layout of database files within the db dir
self._index_path = os.path.join(self._db_dir, 'index.json')
self._verifier_path = os.path.join(self._db_dir, 'index_verifier')
self._lock_path = os.path.join(self._db_dir, 'lock')
# This is for other classes to use to lock prefix directories.
@ -315,6 +322,7 @@ def __init__(self, root, db_dir=None, upstream_dbs=None,
mkdirp(self._failure_dir)
self.is_upstream = is_upstream
self.last_seen_verifier = ''
# initialize rest of state.
self.db_lock_timeout = (
@ -913,6 +921,11 @@ def _write(self, type, value, traceback):
with open(temp_file, 'w') as f:
self._write_to_file(f)
os.rename(temp_file, self._index_path)
if _use_uuid:
with open(self._verifier_path, 'w') as f:
new_verifier = str(uuid.uuid4())
f.write(new_verifier)
self.last_seen_verifier = new_verifier
except BaseException as e:
tty.debug(e)
# Clean up temp file if something goes wrong.
@ -928,8 +941,18 @@ def _read(self):
write lock.
"""
if os.path.isfile(self._index_path):
# Read from file if a database exists
self._read_from_file(self._index_path)
current_verifier = ''
if _use_uuid:
try:
with open(self._verifier_path, 'r') as f:
current_verifier = f.read()
except BaseException:
pass
if ((current_verifier != self.last_seen_verifier) or
(current_verifier == '')):
self.last_seen_verifier = current_verifier
# Read from file if a database exists
self._read_from_file(self._index_path)
return
elif self.is_upstream:
raise UpstreamDatabaseLockingError(
@ -1339,7 +1362,7 @@ def _query(
# TODO: handling of hashes restriction is not particularly elegant.
hash_key = query_spec.dag_hash()
if (hash_key in self._data and
(not hashes or hash_key in hashes)):
(not hashes or hash_key in hashes)):
return [self._data[hash_key].spec]
else:
return []

View file

@ -525,6 +525,8 @@ def database(mock_store, mock_packages, config):
"""This activates the mock store, packages, AND config."""
with use_store(mock_store):
yield mock_store.db
# Force reading the database again between tests
mock_store.db.last_seen_verifier = ''
@pytest.fixture(scope='function')

View file

@ -13,6 +13,12 @@
import os
import pytest
import json
try:
import uuid
_use_uuid = True
except ImportError:
_use_uuid = False
pass
import llnl.util.lock as lk
from llnl.util.tty.colify import colify
@ -469,6 +475,21 @@ def test_015_write_and_read(mutable_database):
assert new_rec.installed == rec.installed
def test_017_write_and_read_without_uuid(mutable_database, monkeypatch):
monkeypatch.setattr(spack.database, '_use_uuid', False)
# write and read DB
with spack.store.db.write_transaction():
specs = spack.store.db.query()
recs = [spack.store.db.get_record(s) for s in specs]
for spec, rec in zip(specs, recs):
new_rec = spack.store.db.get_record(spec)
assert new_rec.ref_count == rec.ref_count
assert new_rec.spec == rec.spec
assert new_rec.path == rec.path
assert new_rec.installed == rec.installed
def test_020_db_sanity(database):
"""Make sure query() returns what's actually in the db."""
_check_db_sanity(database)
@ -703,6 +724,9 @@ def test_old_external_entries_prefix(mutable_database):
with open(spack.store.db._index_path, 'w') as f:
f.write(json.dumps(db_obj))
if _use_uuid:
with open(spack.store.db._verifier_path, 'w') as f:
f.write(str(uuid.uuid4()))
record = spack.store.db.get_record(s)