Repository navigation
Fix race condition in sqlite subscriber - #8758
Open
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 5 commits into
Open
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 5 commits into
Jens Hedegaard Nielsen (jenshnielsen) wants to merge 5 commits into
Conversation
Jens Hedegaard Nielsen (jenshnielsen)
commented
Nov 11, 2026
Jens Hedegaard Nielsen (jenshnielsen)
commented
Collaborator
- Fix race condition in sqlite subscriber -> remove serial test work around + flaky marker from tests
- Raise if subscriber is used together with background writer. This has never worked but would hang. Now it raises cleanly
The tests marked serial were flaky in parallel because of two race conditions in _Subscriber, and one relied on wall-clock timing. - Use the queue size instead of a separate counter that was reset after the callback, which held back rows inserted during a callback until the next insert. - Hold a lock while draining the queue and calling the callback so the completion callback cannot run concurrently with one on the subscriber thread, which delivered results out of order. - Drive the DataSaver clock by hand in test_datasaver_scalars instead of sleeping. With those fixed, drop the serial marker and the separate serial CI step, and drop the flaky marker together with pytest-rerunfailures.
A subscriber's trigger calls an SQL function that is only registered on the DataSet's own connection. The background writer inserts through its own connection, so the first insert failed with "no such function", the writer thread died and the measurement hung waiting for it. This combination has never worked, so reject it explicitly for now. - DataSet.subscribe raises while the database is written in the background, and mark_started(start_bg_writer=True) raises if the dataset already has subscribers, before any start action is taken. - Runner.__enter__ raises before the run is created if subscribers are set up via add_subscriber, the default_subscribers config option or DataSaver.default_callback, so no run is left incomplete.
Jens Hedegaard Nielsen (jenshnielsen)
requested a review
from a team
as a code owner
November 11, 2026 19:18
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8758 +/- ##
=======================================
Coverage 75.42% 75.42%
=======================================
Files 308 308
Lines 32734 32741 +7
=======================================
+ Hits 24689 24696 +7
Misses 8045 8045 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Jens Hedegaard Nielsen (jenshnielsen)
requested a balanced review from Copilot
November 12, 2026 10:03
Copilot started reviewing on behalf of
Jens Hedegaard Nielsen (jenshnielsen)
November 12, 2026 10:03
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The changes incorrectly block subscriptions when an unrelated run writes in the background and introduce a queue race that can deliver rows with an outdated dataset length.
2 open issues
What changed in this PR
This PR addresses SQLite subscriber races in QCoDeS and adds explicit errors for combining subscriptions with background writing.
Changes:
- Serialize subscriber callbacks and replace the separate queue counter with the queue’s size.
- Reject unsupported background-writing subscriptions before measurement runs are created.
- Make timing tests deterministic and remove serial-test and retry workarounds.
| File | Description |
|---|---|
tests/dataset/test_subscribing.py |
Tests background-writing rejection and removes retry/serial markers. |
tests/dataset/test_nested_measurements.py |
Removes retry/serial markers from subscription coverage. |
tests/dataset/test_datasaver.py |
Tests rejection of default callbacks with background writing. |
tests/dataset/measurement/test_measurement_context_manager.py |
Adds early-rejection coverage and replaces sleeps with a controlled clock. |
src/qcodes/dataset/subscriber.py |
Locks callback delivery and uses queue size for dispatch. |
src/qcodes/dataset/measurements.py |
Adds early compatibility checks and documents restrictions. |
src/qcodes/dataset/data_set.py |
Guards subscription and startup against background writing. |
requirements.txt |
Removes the test-rerun dependency. |
pyproject.toml |
Removes the rerun dependency and serial marker registration. |
docs/changes/newsfragments/8758.improved.1 |
Documents background-writing incompatibility errors. |
docs/changes/newsfragments/8758.improved |
Describes subscriber race fixes. |
.github/workflows/pytest.yaml |
Consolidates parallel and serial test execution into one step. |
Review effort: Balanced
| state: Any | None = None, | ||
| callback_kwargs: Mapping[str, Any] | None = None, | ||
| ) -> str: | ||
| if self._writer_status.write_in_background: |
| self.callback(result_list, self._data_set_len, self.state) | ||
| with self._callback_lock: | ||
| result_list = self._exhaust_queue(self.data_queue) | ||
| self.callback(result_list, self._data_set_len, self.state) |
- Only update the length passed to subscriber callbacks while draining the queue under the callback lock. Since the subscriber thread polls the queue size, a row could be delivered before the inserting thread had counted it, so the callback got a stale length. - Only reject subscribing if this run is the one being written in the background. The writer status is shared by every run in a database, so subscribing to a dataset that has not been started was rejected whenever another run was written in the background.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.