Skip to content

Commit 9e39a06

Browse files
committed
Handle SQLite cache errors gracefully instead of aborting upload
Summary: - Fix sqlite3.OperationalError: disk I/O error in the file handle cache from crashing the entire upload process - PersistentCache.get() now catches sqlite3.DatabaseError and returns None (cache miss) instead of propagating the exception - Cache failures are logged as warnings so they remain visible for debugging Context The upload file handle cache is a SQLite database that stores server-returned handles for already-uploaded images. It is purely an optimization to skip re-uploading — if the cache is unavailable, the server's fetch_offset endpoint confirms prior uploads at the cost of one extra HTTP round-trip per image. Previously, any SQLite error during cache lookup (disk I/O error, corrupt database, etc.) propagated up through _continue_or_fail which treated it as a fatal error, aborting the upload of all remaining images. This was observed on Windows with 4 concurrent upload workers rapidly opening/closing SQLite connections to the same cache file.
1 parent abc0056 commit 9e39a06

File tree

2 files changed

+24
-1
lines changed

2 files changed

+24
-1
lines changed

mapillary_tools/history.py

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,12 +118,21 @@ def get(self, key: str) -> str | None:
118118

119119
s = time.perf_counter()
120120

121-
with store.KeyValueStore(self._file, flag="r") as db:
121+
try:
122+
db_ctx = store.KeyValueStore(self._file, flag="r")
123+
except sqlite3.DatabaseError as ex:
124+
LOG.warning(f"Failed to open cache database: {ex}")
125+
return None
126+
127+
with db_ctx as db:
122128
try:
123129
raw_payload: bytes | None = db.get(key) # data retrieved from db[key]
124130
except Exception as ex:
125131
if self._table_not_found(ex):
126132
return None
133+
if isinstance(ex, sqlite3.DatabaseError):
134+
LOG.warning(f"Cache read error for {key}: {ex}")
135+
return None
127136
raise ex
128137

129138
if raw_payload is None:

tests/unit/test_persistent_cache.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,20 @@ def test_keys_read_mode_corrupted_database(tmpdir):
372372
assert all(isinstance(key, str) for key in keys)
373373

374374

375+
def test_get_returns_none_on_corrupt_database_file(tmpdir):
376+
"""Test that get() returns None instead of raising when the database is corrupt."""
377+
cache_file = os.path.join(tmpdir, "cache_corrupt")
378+
cache = PersistentCache(cache_file)
379+
380+
cache.set("key1", "value1")
381+
assert cache.get("key1") == "value1"
382+
383+
with open(cache_file, "wb") as f:
384+
f.write(b"this is not a valid sqlite database file")
385+
386+
assert cache.get("key1") is None
387+
388+
375389
def test_multithread_shared_cache_comprehensive(tmpdir):
376390
"""Test shared cache instance across multiple threads using get->set pattern.
377391

0 commit comments

Comments
 (0)