feat: split raw data storage into per-dataset SQLite files - #8219
Mikhail Astafev (astafan8) wants to merge 25 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8219 +/- ##
==========================================
+ Coverage 71.99% 72.26% +0.26%
==========================================
Files 305 307 +2
Lines 32021 32397 +376
==========================================
+ Hits 23055 23413 +358
- Misses 8966 8984 +18 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Added: update_raw_data_paths helperAdded a utility function for users who move their per-dataset raw data files to a new location. This mirrors the pattern used for exported netCDF files. Usage: `python update_raw_data_paths( The function:
4 tests added, documentation updated in introduction.rst and Database.ipynb. |
Implement a helper function that updates the raw_data_db_path metadata in the main database when individual raw data SQLite files have been moved to a new location. This mirrors the existing pattern used for exported netCDF files. The function: - Scans all datasets in the main DB that have raw_data_db_path metadata - For each, checks if the corresponding .db file exists in the new folder - Updates the stored path in the metadata to point to the new location - Reports which datasets were updated and which were skipped (file missing) Exposed as qcodes.dataset.update_raw_data_paths() for user convenience. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Apply ruff format to _raw_data_storage.py (line length adjustments) - Comment out unused import in Database.ipynb example cell Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add assert statements to narrow path_to_db from str | None to str before passing to update_raw_data_paths(). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two new functions for managing per-dataset raw data storage: - purge_orphaned_datasets(): Finds and removes dataset records from the main DB whose raw data files no longer exist on disk. Useful after archiving/deleting selected raw data files. Defaults to dry_run=True. - cleanup_datasets(): Removes datasets (both DB records AND raw data files) matching criteria: older_than_days, sample_name (exact match), or larger_than_mb. Criteria use AND logic. Defaults to dry_run=True. Both functions return dataclass results with full details of what was found/removed. The _remove_dataset_from_db helper correctly deletes from runs, layouts, dependencies tables and drops the results table. Documentation added to introduction.rst and Database.ipynb. 9 new tests covering both functions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…implify - Move _get_datasets_with_raw_data and _remove_dataset_from_db to src/qcodes/dataset/sqlite/queries.py as public functions - Add print() statements alongside logging for user visibility - Use 'with closing(conn):' pattern for connection management - Simplify orphan detection (rely on raw_data_size_bytes is None) - Simplify cleanup_datasets docstring - Simplify introduction.rst to reference Database notebook - Update news fragment with config option name and DB context Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Change per-dataset log.info to log.debug in purge/cleanup/update_paths - Replace os.remove with Path.unlink() for robustness - Remove unused os import Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- _extract_single_dataset_into_db: use dataset._data_conn instead of dataset.conn so raw data is read from the per-dataset file - unsubscribe/unsubscribe_all: use _data_conn to remove triggers that were created on the raw data connection - Add tests for extract_runs_into_db and export_datasets_and_create_metadata_db with split raw data Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Mikhail Astafev <astafan8@gmail.com>
- Extract connect_to_sqlite_file() and _register_numpy_sqlite_adapters_and_converters() in sqlite/database.py; use them in both connect() and connect_to_raw_data_db() so the numpy adapters and connection settings are defined once - Reuse _create_run_table() in create_raw_data_db() instead of duplicating the results-table CREATE TABLE logic (also gains table-name validation) - Validate table name via _validate_table_name() before DROP TABLE in remove_dataset_from_db() - Apply doc review suggestions and fix module path in dataset_design.rst Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Previously a results table was still created (empty) in the main database for split-storage datasets, purely to satisfy code paths that assumed the table exists. This removes that table entirely so the main DB holds only metadata - cleaner and future-proof for non-sqlite raw data backends. Changes: - Do not create the results table in the main DB when raw storage is enabled (create_run_table=False, insert_into_results_table=False), mirroring how DataSetInMem records runs. - Identify split-storage runs via a dedicated 'raw_data_db_path' column in the runs table, recorded at dataset creation. This disambiguates them from DataSetInMem runs (which also have no results table) in _get_datasetprotocol_from_guid, replacing the old reliance on the empty table's presence. - Keep 'raw_data_db_path' out of the user-facing metadata dict (read/write via get/set_raw_data_db_path_for_run); this also stops it leaking into metadata comparisons and extract/export copies. - number_of_results/__len__ return 0 when the results table does not exist (e.g. a pristine, not-yet-started split dataset). - Defer subscriptions requested before the dataset is started until start time, when the raw data table exists (fixes subscriber trigger creation). - Restrict management queries (purge/cleanup) to started runs only. - Docs + tests updated; add tests for subscribe-before-start, count before start, no-main-table, and extract not carrying over raw_data_db_path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review: factor the subscriber creation into _create_and_start_subscriber (reused by subscribe and _start_pending_subscribers) and the pending-subscription bookkeeping into _queue_pending_subscriber. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…s for query result - Rename connect_to_sqlite_file -> _connect_to_sqlite_file (internal helper, not part of the public API) - Return a RawDataDatasetRecord dataclass from get_datasets_with_raw_data_path instead of an opaque 8-tuple Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add noqa: BLE001 with rationale for the intentional broad excepts in purge_orphaned_datasets / cleanup_datasets that collect per-dataset errors - Collapse nested/duplicate branches flagged by SIM in cleanup_datasets and _get_datasetprotocol_from_guid Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- remove_dataset_from_db: drop the redundant result_table_name argument and look it up from the runs table internally. - Convert the record types RawDataDatasetRecord and DatasetInfo from dataclasses to NamedTuples (lighter immutable value records; the mutable result aggregates PurgeResult/CleanupResult stay dataclasses). - update_raw_data_paths: use closing(conn) so the connection is always closed; fix a docstring typo. - Add tqdm progress bars to the I/O-bound loops: dataset scanning (_build_dataset_info_list), path updates, and the purge/cleanup removals. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move the split-storage behaviour out of DataSet's is_raw_data_storage_enabled() conditionals into a composed ResultsBackend strategy, chosen inside DataSet.__init__ (from config for new runs, from the run's recorded raw_data_db_path for existing runs, so DataSet(run_id=...) auto-detects). - New _results_backend.py: ResultsBackend base (MainDatabaseResultsBackend) plus SeparateSqliteFileResultsBackend. Backends expose a single results_conn and encapsulate the results-table operations (exists/count/length/insert/ read) and a results_db_path used to route background writes. - DataSet delegates to self._results_backend; the confusing _data_conn / _raw_data_conn / _raw_data_db_path trio is replaced by a single _results_conn. data_set_cache.py, subscriber.py and database_extract_runs.py use it too. - Tests assert backend selection and that DataSet(run_id=...) auto-detects, and that split-storage background writes land in the per-dataset file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
987868c to
4a73584
Compare
|
One or more custom setup steps configured for this repository failed during this Copilot code review run: Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review. Note You can configure setup steps for Copilot code review separately from Copilot cloud agent with a |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Backend compatibility, read-only handling, SQL identifier quoting, unsubscription, and destructive cleanup behavior have unresolved correctness issues.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 3
Open (11)
Quote parameter identifiers when creating split raw tables · New Prevent raw-data loss when database removal fails · New Avoid inferring backend from user-defined metadata · New Normalize relative raw data folders before persisting · New Clear pending subscribers in unsubscribe_all · New Propagate read-only mode to split raw-data connections · New Use _results_conn as the documented routing property · New Stop describing raw_data_db_path as metadata · New Correct split storage and raw path documentation · New Document raw_data_db_path as an internal column · New Restore missing test method boundary · New
Resolved since last review (8)
Before interpolating and executing a DROP TABLE statement, validate the table name using the… This section repeats the same pointer to the Database notebook twice; it can be collapsed into a… The design doc refers to a non-existent module path (qcodes.dataset.raw_data_storage). The… Avoid printing to stdout from library helpers; callers can inspect the returned CleanupResult… Avoid printing to stdout from library helpers; logging at INFO is already done here, so the print… Avoid printing to stdout from library helpers; callers can inspect the returned PurgeResult and/or… Avoid printing to stdout from library helpers; this makes programmatic use noisy and hard to… Docstring typo: "old paths3" should be "old paths".
| # Reuse the same results-table creation logic as the main database so | ||
| # the raw-data table schema (column definitions, table-name validation) | ||
| # stays consistent with the rest of QCoDeS. | ||
| _create_run_table(conn, table_name, paramspecs or None) |
| if ds_info.raw_data_db_path: | ||
| raw_path = Path(ds_info.raw_data_db_path) | ||
| if raw_path.is_file(): | ||
| file_size = raw_path.stat().st_size | ||
| raw_path.unlink() |
There was a problem hiding this comment.
Fixed in ed03933 - cleanup now removes the DB record first and only then unlinks the raw file, so a DB failure cannot leave a run pointing at deleted data.
| From a design perspective, this feature adds a thin routing layer inside the | ||
| ``DataSet`` class without changing any public interfaces: | ||
|
|
||
| - A ``_data_conn`` property transparently returns either the main database |
There was a problem hiding this comment.
Fixed in ed03933 - corrected to _results_conn and refreshed the design section to describe the pluggable ResultsBackend.
| new_raw_data_folder="/new/location/of/raw_files/" | ||
| ) | ||
|
|
||
| This scans all datasets with a ``raw_data_db_path`` metadata entry, checks whether the corresponding ``.db`` file exists in the new folder, and updates the stored path accordingly. |
There was a problem hiding this comment.
Fixed in ed03933 - now described as the internal raw_data_db_path runs column instead of metadata.
| "3. The main database retains the results table schema (column definitions) but contains no data rows, keeping it small.\n", | ||
| "4. The path to the per-dataset file is saved in the run metadata, so `load_by_id()` and related functions automatically find and reconnect to the correct file.\n", |
There was a problem hiding this comment.
Fixed in ed03933 - the notebook now states that no results table is created in the main DB for split datasets and that raw_data_db_path is an internal runs column excluded from user-facing metadata.
| The function scans all runs that have a ``raw_data_db_path`` metadata | ||
| entry, verifies that a file with the expected GUID-based name exists in |
There was a problem hiding this comment.
Fixed in ed03933 - the docstring now describes raw_data_db_path as an internal runs column rather than a metadata entry.
| assert len(received) > 0 | ||
| assert received[-1] == 5 | ||
| self._close_ds(ds) | ||
| """get_parameter_data should read from the raw data file.""" |
There was a problem hiding this comment.
Fixed in ed03933 - restored the missing test_get_parameter_data_from_raw_data method boundary so the two behaviours are collected as independent tests again.
There was a problem hiding this comment.
don't mention manual editing of qcodesrc.json, the qcodes configuration tutorial/article documents how to make changes and ideally via python interface, so remove this or add a reference to qcodes config page.
There was a problem hiding this comment.
- The main database retains the results table schema (column definitions) but contains no data rows, keeping it small.
this is not true anymore, is it? please go thorugh the docs in the PR and ensure they are true to the state of the latest implementation
…, robustness - Replace the `creates_results_table_in_main_db` boolean with a polymorphic `ResultsBackend.create_results_table` (and `setup_on_new_run`) so each backend owns where its results table is created; the main backend still creates the (empty) table up front to keep DataSet/DataSetInMem disambiguation and behaviour unchanged. - Thread `read_only` through the public loaders into `DataSet` so split datasets loaded read-only open their per-dataset file read-only too. - `unsubscribe_all` now also clears deferred pending subscribers. - `update_raw_data_paths` resolves the new folder to an absolute path. - cleanup removes the DB record before unlinking the raw file (no data loss on DB failure). - Restore the lost `test_get_parameter_data_from_raw_data` method boundary. - Docs/docstrings: describe raw_data_db_path as an internal runs column (not metadata); fix _results_conn name and design section; remove a stale comment.
The results-backend refactor split results-table creation across setup_on_new_run and create_results_table, roughly doubling the number of SQLite commits (fsyncs) in the per-dataset start path versus main. Batch the add_parameter registration loop and the backend column creation each into a single transaction so dataset setup performs one commit per phase instead of one per parameter. This restores measurement throughput to parity with main (realistic 1500-point measurements: was ~+8%, now within noise).
|
|
||
| def setup_on_start(self) -> None: | ||
| """Create/open the results store when the run is started. No-op here.""" | ||
| def create_results_table(self) -> None: |
There was a problem hiding this comment.
i think it is confusing the there's a "create_results_table" mthod on the backend which is different from setup_on_new_run and ther's also this _create_run_table private function. can't this create_results_table be part of setup_on_new_run or setup_on_start? we might have backends in the future that don't create results tables at all :)
There was a problem hiding this comment.
Good point - fixed in 0c88bad. Renamed the hook from create_results_table to the generic setup_on_start (paired with setup_on_new_run), so the lifecycle no longer presumes a "results table". A future backend that stores data in another format just implements setup_on_new_run/setup_on_start and does whatever its storage needs there. The low-level _create_run_table is the existing qcodes helper for the SQLite backends only and is now called from within those two hooks, not exposed as a backend concept.
Separately (per your other request) the on/off boolean is gone: backends are now selected by name via dataset.raw_data_backend (sqlite_main_db | sqlite_per_dataset_db) with per-backend settings under dataset.raw_data_backend_config.
Config: replace the dataset.raw_data_to_separate_db boolean and top-level
dataset.raw_data_path with a dataset.raw_data_backend enum
('sqlite_main_db' | 'sqlite_per_dataset_db') plus a dataset.raw_data_backend_config
object holding per-backend settings (the per-dataset backend's raw_data_path
lives there). Backends are registered by their backend_name, so adding a
backend is a matter of a new ResultsBackend subclass plus a raw_data_backend
enum value and raw_data_backend_config section.
Lifecycle: rename the backend's create_results_table hook to the generic
setup_on_start (paired with setup_on_new_run), so the abstraction does not
presume a 'results table' - a future backend that stores data in another
format just implements the lifecycle hooks. Behaviour is unchanged.
Also rename the internal background-writer queue key raw_data_path to
results_db_path to avoid confusion with the config path. Updates tests,
docs, docstrings and the newsfragment.



Summary
Opt-in split raw data storage: each dataset's raw measurement data (its results table) can be written to an individual per-dataset SQLite file, while all metadata stays in the main database. This keeps the main DB small instead of growing to many gigabytes as datasets accumulate. The public
DataSetAPI is unchanged — every method behaves identically whichever backend is used.Selected via
datasetconfig options inqcodesrc.json:raw_data_backend(string, default"sqlite_main_db") — the results backend; set to"sqlite_per_dataset_db"to enable split storage.raw_data_backend_config(object) — per-backend settings keyed by backend name. The per-dataset backend readsraw_data_backend_config.sqlite_per_dataset_db.raw_data_path(default"{db_location}", expanded relative to the main DB path), the folder for the per-dataset files.How it works
ResultsBackendstrategy (src/qcodes/dataset/_results_backend.py) encapsulates where/how a dataset's results are stored, soDataSetcarries no storage-specific conditionals. Backends are registered by theirbackend_name(theraw_data_backendconfig value):MainDatabaseResultsBackend(sqlite_main_db, default) andSeparateSqliteFileResultsBackend(sqlite_per_dataset_db). The backend is chosen insideDataSet.__init__— from config for new runs, and from the run's recorded state for existing runs, so a plainDataSet(run_id=...)auto-detects. Everything routes through the singleresults_conn; the backend also owns the results-table operations (exists/count/length/insert/read). Adding a backend is a new subclass plus araw_data_backendenum value and araw_data_backend_configsection.setup_on_load,setup_on_new_run,setup_on_start,close— so the abstraction never presumes a "results table"; a future backend that stores data in another format just implements the hooks.<guid>.db, containing only the results table + numpy type adapters (no metadata schema). Created via shared helpers_connect_to_sqlite_file()/_create_run_table()so schema and connection settings match the main DB (also used to dedupconnect()).sqlite_per_dataset_db, no results table is created there — only run metadata. A split run is marked by a dedicatedraw_data_db_pathcolumn inruns(recorded at dataset creation), which also distinguishes it from aDataSetInMemrun when loading. That column is an internal detail: it is excluded from the user-facingmetadatadict (accessed viaget/set_raw_data_db_path_for_run), so it never leaks intothe_same_dataset_ascomparisons or extracted/exported DBs.number_of_results/__len__return0when the results table doesn't exist yet (pristine dataset). Subscriptions requested before start are deferred and materialised on start;read_onlyis threaded through the public loaders so a read-only load opens the per-dataset file read-only too. Extract/export work unchanged (extract_runs_into_dbreads via the backend and doesn't carry overraw_data_db_path). Per-dataset setup writes are batched into single transactions, keeping measurement throughput on par with the default backend.Management helpers
Exposed via
qcodes.dataset(both destructive ones default todry_run=True, show atqdmprogress bar, and return a result object; deletion reuses the existingremove_dataset_from_db):update_raw_data_paths(db_path, new_raw_data_folder)— fix stored paths after moving the per-dataset files.purge_orphaned_datasets(db_path, *, dry_run=True)— remove main-DB records whose raw data file is gone.cleanup_datasets(db_path, *, older_than_days=None, sample_name=None, larger_than_mb=None, dry_run=True)— remove datasets (records + files) matching the given criteria (AND).Files
_results_backend.py(the backend strategy + registry);_raw_data_storage.py(storage + config accessors + management helpers);tests/dataset/test_raw_data_storage.py;docs/changes/newsfragments/8219.new.data_set.py(delegate to the backend, no-empty-table, deferred subscribers, disambiguation);sqlite/queries.py(get/set_raw_data_db_path_for_run,get_datasets_with_raw_data_path+RawDataDatasetRecord,remove_dataset_from_db, column excluded from metadata);sqlite/database.py(shared connect helpers);data_set_cache.py,subscriber.py,database_extract_runs.py(useresults_conn);__init__.py(exports);qcodesrc.json+qcodesrc_schema.json(config); docs.Verification
test_raw_data_storage.pycovers unit, integration, management, extract/export, config/backend selection +DataSet(run_id=...)auto-detection, and the no-empty-table / count-before-start / subscribe-before-start / background-write cases. Fulltests/datasetsuite passes with no regressions in default mode. Pyright, ruff and pre-commit all clean.