Add persistent caching support for index classes - #88
Conversation
- Add sqlitedict dependency to pyproject.toml and update app.requirements.txt - Replace in-memory _IndexCache dictionaries with SqliteDict
b6e10b3 to
c9b3adb
Compare
mateo-velez
changed the title
There was a problem hiding this comment.
Why are you removing the hashes? Don't bother updating the Docker image app dependency pins here, I'll do it in a separate commit.
| "jinja2 ~= 3.0", | ||
| "lxml >= 4.8, < 7.0", | ||
| "requests ~= 2.27", | ||
| "sqlitedict >= 2.0, < 3.0", |
There was a problem hiding this comment.
| "sqlitedict >= 2.0, < 3.0", |
Don't introduce a third-party dependency; use sqlite3 instead
| return | ||
| self._index_t = None | ||
| self._index = {} | ||
| with self._index_lock: |
There was a problem hiding this comment.
Don't remove the no-op for concurrent calls to invalidate_list
| logger.info(f"Project '{name}' files already undergoing update") | ||
| return | ||
| self._packages.pop(package_name, None) | ||
| with self._package_locks[name]: |
There was a problem hiding this comment.
Don't remove the no-op for concurrent calls to invalidate_project
There was a problem hiding this comment.
Got it. However, it's worth pointing out that the method may return as if the invalidation has completed, while another concurrent call is still in progress.
There was a problem hiding this comment.
This is true. The intention is to not immediately invalidate after the index is updated
| ) | ||
| self._index_lock = threading.Lock() | ||
| self._index_metadata = sqlitedict.SqliteDict( | ||
| self.cache_file, |
There was a problem hiding this comment.
cache_file should be an input argument (and environment variable). If not set, the original in-memory caching should be used instead
- Revert app.requirements.txt and pyproject.toml - Add no-op to invalidation methods for the index cache class - Add optional env var PROXPI_INDEX_CACHE_DIR to enable persistent index caching
b64659a to
dc40817
Compare
|
I'm considering adding some unit testing for that specific new class, if I get the green flag ofc. |
|
|
||
| - Keys are expected to be strings. | ||
| - Thread-safety must be handled by the caller. | ||
| """ |
There was a problem hiding this comment.
| """ | |
| Heavily inspired by 'sqlitedict': https://github.com/piskvorky/sqlitedict | |
| """ |
Reference source
| def __init__(self, path: str): | ||
| self.conn = sqlite3.connect(path, check_same_thread=False) | ||
| self.conn.execute("PRAGMA synchronous = NORMAL;") | ||
| self.conn.execute("PRAGMA journal_mode = WAL;") | ||
|
|
||
| self.table_name = "t" | ||
| self.encode = pickle.dumps | ||
| self.decode = pickle.loads | ||
|
|
||
| self.conn.execute( | ||
| f"CREATE TABLE IF NOT EXISTS {self.table_name} (key BLOB PRIMARY KEY, value BLOB)" | ||
| ) |
There was a problem hiding this comment.
| def __init__(self, path: str): | |
| self.conn = sqlite3.connect(path, check_same_thread=False) | |
| self.conn.execute("PRAGMA synchronous = NORMAL;") | |
| self.conn.execute("PRAGMA journal_mode = WAL;") | |
| self.table_name = "t" | |
| self.encode = pickle.dumps | |
| self.decode = pickle.loads | |
| self.conn.execute( | |
| f"CREATE TABLE IF NOT EXISTS {self.table_name} (key BLOB PRIMARY KEY, value BLOB)" | |
| ) | |
| def __init__(self, conn: sqlite3.Connection, table_name: str) -> None: | |
| self.conn = conn | |
| self.table_name = table_name | |
| self.encode = pickle.dumps | |
| self.decode = pickle.loads | |
| def ensure_initialised(self) -> None: | |
| self.conn.execute( | |
| f"CREATE TABLE IF NOT EXISTS {self.table_name} (key BLOB PRIMARY KEY, value BLOB)" | |
| ) |
- I think it's better to have one database (file), with multiple tables
- Don't run complex code inside
__init__(makes unit testing much simpler)
There was a problem hiding this comment.
- One file per table allow us to concurrently perform modification of package and index tables. Although, one file is indeed simpler and cleaner.
There was a problem hiding this comment.
It's unlikely that we have modification of package and index caches at the same time, to the point where I prefer the simplicity of the code.
…le and improve connection management
2 participants
Issue: #87