Add ability to disable the sqlite3 quick_check (#39479)

This commit is contained in:
J. Nick Koston 2020-09-02 03:12:56 -05:00 committed by GitHub
parent 225becc89a
commit 557684c3ce
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 99 additions and 21 deletions

View file

@ -35,7 +35,7 @@ from homeassistant.helpers.typing import ConfigType
import homeassistant.util.dt as dt_util
from . import migration, purge
from .const import DATA_INSTANCE, DOMAIN, SQLITE_URL_PREFIX
from .const import CONF_DB_INTEGRITY_CHECK, DATA_INSTANCE, DOMAIN, SQLITE_URL_PREFIX
from .models import Base, Events, RecorderRuns, States
from .util import session_scope, validate_or_move_away_sqlite_database
@ -55,6 +55,7 @@ SERVICE_PURGE_SCHEMA = vol.Schema(
DEFAULT_URL = "sqlite:///{hass_config_path}"
DEFAULT_DB_FILE = "home-assistant_v2.db"
DEFAULT_DB_INTEGRITY_CHECK = True
DEFAULT_DB_MAX_RETRIES = 10
DEFAULT_DB_RETRY_WAIT = 3
KEEPALIVE_TIME = 30
@ -99,6 +100,9 @@ CONFIG_SCHEMA = vol.Schema(
vol.Optional(
CONF_DB_RETRY_WAIT, default=DEFAULT_DB_RETRY_WAIT
): cv.positive_int,
vol.Optional(
CONF_DB_INTEGRITY_CHECK, default=DEFAULT_DB_INTEGRITY_CHECK
): cv.boolean,
}
),
)
@ -156,6 +160,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
commit_interval = conf[CONF_COMMIT_INTERVAL]
db_max_retries = conf[CONF_DB_MAX_RETRIES]
db_retry_wait = conf[CONF_DB_RETRY_WAIT]
db_integrity_check = conf[CONF_DB_INTEGRITY_CHECK]
db_url = conf.get(CONF_DB_URL)
if not db_url:
@ -172,6 +177,7 @@ async def async_setup(hass: HomeAssistant, config: ConfigType) -> bool:
db_retry_wait=db_retry_wait,
entity_filter=entity_filter,
exclude_t=exclude_t,
db_integrity_check=db_integrity_check,
)
instance.async_initialize()
instance.start()
@ -204,6 +210,7 @@ class Recorder(threading.Thread):
db_retry_wait: int,
entity_filter: Callable[[str], bool],
exclude_t: List[str],
db_integrity_check: bool,
) -> None:
"""Initialize the recorder."""
threading.Thread.__init__(self, name="Recorder")
@ -217,6 +224,7 @@ class Recorder(threading.Thread):
self.db_url = uri
self.db_max_retries = db_max_retries
self.db_retry_wait = db_retry_wait
self.db_integrity_check = db_integrity_check
self.async_db_ready = asyncio.Future()
self.engine: Any = None
self.run_info: Any = None
@ -547,7 +555,9 @@ class Recorder(threading.Thread):
# On systems with very large databases and
# very slow disk or cpus, this can take a while.
#
validate_or_move_away_sqlite_database(self.db_url)
validate_or_move_away_sqlite_database(
self.db_url, self.db_integrity_check
)
if self.engine is not None:
self.engine.dispose()

View file

@ -3,3 +3,5 @@
DATA_INSTANCE = "recorder_instance"
SQLITE_URL_PREFIX = "sqlite://"
DOMAIN = "recorder"
CONF_DB_INTEGRITY_CHECK = "db_integrity_check"

View file

@ -9,7 +9,7 @@ from sqlalchemy.exc import OperationalError, SQLAlchemyError
import homeassistant.util.dt as dt_util
from .const import DATA_INSTANCE, SQLITE_URL_PREFIX
from .const import CONF_DB_INTEGRITY_CHECK, DATA_INSTANCE, SQLITE_URL_PREFIX
from .models import ALL_TABLES, process_timestamp
_LOGGER = logging.getLogger(__name__)
@ -21,7 +21,7 @@ SQLITE3_POSTFIXES = ["", "-wal", "-shm"]
# This is the maximum time after the recorder ends the session
# before we no longer consider startup to be a "restart" and we
# should do a check on the sqlite3 database.
MAX_RESTART_TIME = timedelta(minutes=6)
MAX_RESTART_TIME = timedelta(minutes=10)
@contextmanager
@ -110,7 +110,7 @@ def execute(qry, to_native=False, validate_entity_ids=True):
time.sleep(QUERY_RETRY_WAIT)
def validate_or_move_away_sqlite_database(dburl: str) -> bool:
def validate_or_move_away_sqlite_database(dburl: str, db_integrity_check: bool) -> bool:
"""Ensure that the database is valid or move it away."""
dbpath = dburl[len(SQLITE_URL_PREFIX) :]
@ -118,7 +118,7 @@ def validate_or_move_away_sqlite_database(dburl: str) -> bool:
# Database does not exist yet, this is OK
return True
if not validate_sqlite_database(dbpath):
if not validate_sqlite_database(dbpath, db_integrity_check):
_move_away_broken_database(dbpath)
return False
@ -154,13 +154,13 @@ def basic_sanity_check(cursor):
return True
def validate_sqlite_database(dbpath: str) -> bool:
def validate_sqlite_database(dbpath: str, db_integrity_check: bool) -> bool:
"""Run a quick check on an sqlite database to see if it is corrupt."""
import sqlite3 # pylint: disable=import-outside-toplevel
try:
conn = sqlite3.connect(dbpath)
run_checks_on_open_db(dbpath, conn.cursor())
run_checks_on_open_db(dbpath, conn.cursor(), db_integrity_check)
conn.close()
except sqlite3.DatabaseError:
_LOGGER.exception("The database at %s is corrupt or malformed.", dbpath)
@ -169,7 +169,7 @@ def validate_sqlite_database(dbpath: str) -> bool:
return True
def run_checks_on_open_db(dbpath, cursor):
def run_checks_on_open_db(dbpath, cursor, db_integrity_check):
"""Run checks that will generate a sqlite3 exception if there is corruption."""
if basic_sanity_check(cursor) and last_run_was_recently_clean(cursor):
_LOGGER.debug(
@ -177,6 +177,16 @@ def run_checks_on_open_db(dbpath, cursor):
)
return
if not db_integrity_check:
# Always warn so when it does fail they remember it has
# been manually disabled
_LOGGER.warning(
"The quick_check on the sqlite3 database at %s was skipped because %s was disabled",
dbpath,
CONF_DB_INTEGRITY_CHECK,
)
return
_LOGGER.debug(
"A quick_check is being performed on the sqlite3 database at %s", dbpath
)

View file

@ -304,6 +304,7 @@ def test_recorder_setup_failure():
db_retry_wait=3,
entity_filter=CONFIG_SCHEMA({DOMAIN: {}}),
exclude_t=[],
db_integrity_check=False,
)
rec.start()
rec.join()

View file

@ -69,29 +69,77 @@ def test_recorder_bad_execute(hass_recorder):
assert e_mock.call_count == 2
def test_validate_or_move_away_sqlite_database(hass, tmpdir, caplog):
"""Ensure a malformed sqlite database is moved away."""
def test_validate_or_move_away_sqlite_database_with_integrity_check(
hass, tmpdir, caplog
):
"""Ensure a malformed sqlite database is moved away.
A quick_check is run here
"""
db_integrity_check = True
test_dir = tmpdir.mkdir("test_validate_or_move_away_sqlite_database")
test_db_file = f"{test_dir}/broken.db"
dburl = f"{SQLITE_URL_PREFIX}{test_db_file}"
util.validate_sqlite_database(test_db_file) is True
util.validate_sqlite_database(test_db_file, db_integrity_check) is True
assert os.path.exists(test_db_file) is True
assert util.validate_or_move_away_sqlite_database(dburl) is False
assert (
util.validate_or_move_away_sqlite_database(dburl, db_integrity_check) is False
)
_corrupt_db_file(test_db_file)
assert util.validate_sqlite_database(dburl) is False
assert util.validate_sqlite_database(dburl, db_integrity_check) is False
assert util.validate_or_move_away_sqlite_database(dburl) is False
assert (
util.validate_or_move_away_sqlite_database(dburl, db_integrity_check) is False
)
assert "corrupt or malformed" in caplog.text
assert util.validate_sqlite_database(dburl) is False
assert util.validate_sqlite_database(dburl, db_integrity_check) is False
assert util.validate_or_move_away_sqlite_database(dburl) is True
assert util.validate_or_move_away_sqlite_database(dburl, db_integrity_check) is True
def test_validate_or_move_away_sqlite_database_without_integrity_check(
hass, tmpdir, caplog
):
"""Ensure a malformed sqlite database is moved away.
The quick_check is skipped, but we can still find
corruption if the whole database is unreadable
"""
db_integrity_check = False
test_dir = tmpdir.mkdir("test_validate_or_move_away_sqlite_database")
test_db_file = f"{test_dir}/broken.db"
dburl = f"{SQLITE_URL_PREFIX}{test_db_file}"
util.validate_sqlite_database(test_db_file, db_integrity_check) is True
assert os.path.exists(test_db_file) is True
assert (
util.validate_or_move_away_sqlite_database(dburl, db_integrity_check) is False
)
_corrupt_db_file(test_db_file)
assert util.validate_sqlite_database(dburl, db_integrity_check) is False
assert (
util.validate_or_move_away_sqlite_database(dburl, db_integrity_check) is False
)
assert "corrupt or malformed" in caplog.text
assert util.validate_sqlite_database(dburl, db_integrity_check) is False
assert util.validate_or_move_away_sqlite_database(dburl, db_integrity_check) is True
def test_last_run_was_recently_clean(hass_recorder):
@ -134,25 +182,32 @@ def test_combined_checks(hass_recorder):
"""Run Checks on the open database."""
hass = hass_recorder()
db_integrity_check = False
cursor = hass.data[DATA_INSTANCE].engine.raw_connection().cursor()
assert util.run_checks_on_open_db("fake_db_path", cursor) is None
assert (
util.run_checks_on_open_db("fake_db_path", cursor, db_integrity_check) is None
)
# We are patching recorder.util here in order
# to avoid creating the full database on disk
with patch("homeassistant.components.recorder.util.last_run_was_recently_clean"):
assert util.run_checks_on_open_db("fake_db_path", cursor) is None
assert (
util.run_checks_on_open_db("fake_db_path", cursor, db_integrity_check)
is None
)
with patch(
"homeassistant.components.recorder.util.last_run_was_recently_clean",
side_effect=sqlite3.DatabaseError,
), pytest.raises(sqlite3.DatabaseError):
util.run_checks_on_open_db("fake_db_path", cursor)
util.run_checks_on_open_db("fake_db_path", cursor, db_integrity_check)
cursor.execute("DROP TABLE events;")
with pytest.raises(sqlite3.DatabaseError):
util.run_checks_on_open_db("fake_db_path", cursor)
util.run_checks_on_open_db("fake_db_path", cursor, db_integrity_check)
def _corrupt_db_file(test_db_file):