Skip to content

Preserve live-query re-evaluation when writes race _execOverDatabase()#8536

Merged
pubkey merged 6 commits into
masterfrom
copilot/test-performance-v16-v17
May 18, 2026
Merged

Preserve live-query re-evaluation when writes race _execOverDatabase()#8536
pubkey merged 6 commits into
masterfrom
copilot/test-performance-v16-v17

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

Live queries could stall or miss the required follow-up evaluation when a write committed while _execOverDatabase() was still reading. The previous retry-with-promiseWait(rerunCount * 20) approach avoided a tight loop, but under sustained writes the growing backoff could keep deferring progress and delay the first query.$ emission.

  • Problem

    • _execOverDatabase() could finish with a counter that had already advanced past a concurrent write event.
    • A retry loop with delay tied to rerunCount could keep stretching out re-execution under write pressure.
  • Counter handling

    • Snapshot the ChangeEventBuffer counter before running the storage read.
    • If the counter changes during the read, treat that as a concurrent write and re-run the query.
  • Retry behavior

    • Keep the internal rerun path in _execOverDatabase().
    • Change the retry delay to a fixed promiseWait(20) instead of promiseWait(rerunCount * 20).
    • This still yields on fast non-async storages like in-memory, without compounding the delay on every rerun.
  • Regression coverage

    • Keep the race test that stalls query() and commits a write while the initial query is in flight, verifying that query.$ still emits the inserted document.
const counterBefore = this.collection._changeEventBuffer.getCounter();
const queryResult = await queryCollection<RxDocType>(this as any);

if (this.collection._changeEventBuffer.getCounter() !== counterBefore) {
    await promiseWait(20);
    return this._execOverDatabase(rerunCount + 1);
}

Copilot AI and others added 2 commits May 6, 2026 18:43
…ore signal for eventual consistency

Agent-Logs-Url: https://github.com/pubkey/rxdb/sessions/e3b3eb19-a9bb-4a2e-ade7-4a0d97dab92f

Co-authored-by: pubkey <8926560+pubkey@users.noreply.github.com>
…rent writes

Agent-Logs-Url: https://github.com/pubkey/rxdb/sessions/18b5bac4-27b1-4b4f-a295-1f30c3108f7a

Co-authored-by: pubkey <8926560+pubkey@users.noreply.github.com>
@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 18, 2026

@copilot what was the problem? I stilil think doing a promiseWait a bit is better to ensure on non-async storages like in-memory we do not full-block the javasciript process.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

@copilot what was the problem? I stilil think doing a promiseWait a bit is better to ensure on non-async storages like in-memory we do not full-block the javasciript process.

The main problem was not event-loop blocking. The bug was that a write could happen while _execOverDatabase() was in flight, and then the query could return with a counter that made _ensureEqual() think everything was already in sync, so the follow-up re-evaluation was skipped. In the previous version, retrying inside _execOverDatabase() with promiseWait() avoided a tight loop, but under continuous writes it could also keep retrying indefinitely and delay or prevent the first query.$ emission.

So I think a small promiseWait() is fine if we want to yield for fairness on fast non-async storages, but it should be separate from the retry-until-stable logic. The fix here is about correctness: hand the mismatch back to the normal _ensureEqualQueue path so progress is guaranteed even if writes keep coming.

Copilot finished work on behalf of pubkey May 18, 2026 12:02
Copilot AI requested a review from pubkey May 18, 2026 12:02
@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 18, 2026

@copilot ok, use promiseWait(20) no mather which rerunCount we have. still keep the rerunCount in place.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 18, 2026

⚠️ Verify Test Reproduction: Tests PASSED without the fix (unexpected)

The changed tests do not fail without the source changes from this PR. Please inspect whether the test changes actually test the bug that the source changes fix.

This workflow runs the changed tests without the source fix to verify they reproduce the bug.

Show output
...(truncated, showing last 200 of 4131 lines)
    CRUD
      �[32m✓ �[39mshould insert one attachment
      �[32m✓ �[39mshould get the attachment
      �[32m✓ �[39mshould get the data
    compare size
      �[32m✓ �[39mshould have a smaller size when compression is used
    selective compression
      �[32m✓ �[39mshould compress a compressible type (text/plain) and roundtrip correctly
      �[32m✓ �[39mshould NOT compress a non-compressible type (image/jpeg) but still roundtrip correctly
      �[32m✓ �[39mshould compress text but not jpeg on the same document
    isCompressibleType()
      �[32m✓ �[39mshould match wildcard patterns
      �[32m✓ �[39mshould match exact patterns
      �[32m✓ �[39mshould be case-insensitive
      �[32m✓ �[39mshould NOT match non-compressible types
      �[32m✓ �[39mshould match types that include RFC 2045 parameters like charset
    MIME type preservation
      �[32m✓ �[39mfull roundtrip through storage should preserve MIME type on getData()
      �[32m✓ �[39mfull roundtrip should preserve MIME type for non-compressible type

  attachments-compression.test.ts (mode: gzip)
    basics
      �[32m✓ �[39mcompress->decompress
    CRUD
      �[32m✓ �[39mshould insert one attachment
      �[32m✓ �[39mshould get the attachment
      �[32m✓ �[39mshould get the data
    compare size
      �[32m✓ �[39mshould have a smaller size when compression is used
    selective compression
      �[32m✓ �[39mshould compress a compressible type (text/plain) and roundtrip correctly
      �[32m✓ �[39mshould NOT compress a non-compressible type (image/jpeg) but still roundtrip correctly
      �[32m✓ �[39mshould compress text but not jpeg on the same document
    isCompressibleType()
      �[32m✓ �[39mshould match wildcard patterns
      �[32m✓ �[39mshould match exact patterns
      �[32m✓ �[39mshould be case-insensitive
      �[32m✓ �[39mshould NOT match non-compressible types
      �[32m✓ �[39mshould match types that include RFC 2045 parameters like charset
    MIME type preservation
      �[32m✓ �[39mfull roundtrip through storage should preserve MIME type on getData()
      �[32m✓ �[39mfull roundtrip should preserve MIME type for non-compressible type
WARN LOG: �[36m'-------------- RxDB Open Core RxStorage -------------------------------
You are using the free Dexie.js based RxStorage implementation from RxDB https://rxdb.info/rx-storage-dexie.html?console=dexie 
While this is a great option, we want to let you know that there are faster storage solutions available in our premium plugins.
For professional users and production environments, we highly recommend considering these premium options to enhance performance and reliability.
 https://rxdb.info/premium/?console=dexie 
If you already purchased premium access you can disable this log by calling the setPremiumFlag() function from rxdb-premium/plugins/shared.
---------------------------------------------------------------------'�[39m

  migration-storage.test.ts (prev-major to newest (dexie))
    basic migrations
      �[32m✓ �[39mcreate both databases
      �[32m✓ �[39mshould migrate all documents
      �[32m✓ �[39mshould migrate in parallel
      �[32m✓ �[39mmigrate new->new should also work
    issues
      �[32m✓ �[39mmigration with multiple collections

  migration-storage.test.ts (newest to newest)
    basic migrations
      �[32m✓ �[39mcreate both databases
      �[32m✓ �[39mshould migrate all documents
      �[32m✓ �[39mshould migrate in parallel
      �[32m✓ �[39mmigrate new->new should also work
    issues
      �[32m✓ �[39mmigration with multiple collections

  webmcp.test.ts
    �[32m✓ �[39mshould register query tool when registerWebMCP is called
    �[32m✓ �[39mshould wait for changes using wait_changes tool
    �[32m✓ �[39mchanges tool should return documents without internal meta fields
    �[32m✓ �[39mshould iterate over changes using checkpoint
    �[32m✓ �[39mshould execute modifier tools successfully (insert/upsert/delete)
    �[32m✓ �[39mshould unregister tools when collection is closed
    �[32m✓ �[39mshould not register modifier tools when readOnly is true
    �[32m✓ �[39mshould emit log$ and error$ events for executed tools
    �[32m✓ �[39mshould register tools for newly added collections dynamically

  crdt.test.ts
    collection creation
      �[32m✓ �[39mshould throw if the wrong conflict handler is set
      �[32m✓ �[39mshould automatically set the CRDT conflict handler
    .insert()
      �[32m✓ �[39mshould insert a document and initialize the crdt state
      �[32m✓ �[39mshould insert document via bulkInsert
    .insertCRDT()
      �[32m✓ �[39mshould insert the document
      �[32m✓ �[39mshould insert the document with undefined argument
      �[32m✓ �[39mshould respect the if-else logic
    .remove()
      �[32m✓ �[39mshould delete the document via .remove
    .incrementalPatch()
      �[32m✓ �[39mshould update the document
    disallowed methods
      �[32m✓ �[39mshould throw on incrementalModify
      �[32m✓ �[39mshould throw on modify
    redirected methods
      �[32m✓ �[39mshould redirect patch through updateCRDT
      �[32m✓ �[39mshould redirect incrementalRemove through updateCRDT
      �[32m✓ �[39mshould redirect update through updateCRDT
    conflict handling
      init
        �[32m✓ �[39minit
      .getCRDTConflictHandler()
        �[32m✓ �[39mshould merge 2 inserts correctly
        �[32m✓ �[39mshould preserve schema default values during conflict resolution
        �[32m✓ �[39mshould preserve the composite primary key during conflict resolution
      conflicts during replication
        �[32m✓ �[39mshould merge the +1 increments

  population.test.js
    createRxSchema
      positive
        �[32m✓ �[39mshould allow to create a schema with a relation
        �[32m✓ �[39mshould allow primary as relation key
        �[32m✓ �[39mshould allow to create a schema with a relation in nested
        �[32m✓ �[39mshould allow to create relation of array
        �[32m✓ �[39mshould allow to create relation with nullable string
      negative
        �[32m✓ �[39mthrow if ref-type is no string
        �[32m✓ �[39mthrow if ref-type is no string (array)
    RxDocument().populate()
      positive
        �[32m✓ �[39mpopulate top-level-field
        �[32m✓ �[39mpopulate nested field
        �[32m✓ �[39mpopulate string-array
        �[32m✓ �[39mpopulate with primary as ref
      negative
        �[32m✓ �[39mthrow DOC5 for a path that does not exist in the schema, even when the value is falsy
        �[32m✓ �[39mthrow DOC6 when populating a non-ref schema field, even when the value is falsy
    RxDocument populate via pseudo-proxy
      positive
        �[32m✓ �[39mpopulate top-level-field
        �[32m✓ �[39mpopulate nested field
    issues
      �[32m✓ �[39m#222 population not working when multiInstance: false
      �[32m✓ �[39mpopulate array should preserve the order of ref ids when two documents reference the same set in different order
      �[32m✓ �[39mpopulate array when ref is defined on items instead of on the array field

  leader-election.test.js
    .die()
      �[32m✓ �[39mother instance applies on death of leader
    election
      �[32m✓ �[39ma single instance should always elect itself as leader
      �[32m✓ �[39mshould not elect as leader if other instance is leader
      �[32m✓ �[39mwhen 2 instances apply at the same time, one should win
      �[32m✓ �[39mwhen many instances apply, one should win
      �[32m✓ �[39mwhen the leader dies, a new one should be elected
    cleanup
      �[32m✓ �[39mshould properly call die() on the elector when the database is closed
    integration
      �[32m✓ �[39mnon-multiInstance should always be leader
      �[32m✓ �[39mnon-multiInstance: waitForLeadership should instant
      �[32m✓ �[39mwaitForLeadership: run once when instance becomes leader

  import-export.test.js
    Collection
      .exportJSON()
        �[32m✓ �[39mexport the collection
        �[32m✓ �[39mexport encrypted as decrypted
      .importJSON()
        positive
          �[32m✓ �[39mimport json
        negative
          �[32m✓ �[39mshould not import if schema is different
    Database
      .exportJSON()
        �[32m✓ �[39mshould export a valid dump
        �[32m✓ �[39mexport encrypted as decrypted
        �[32m✓ �[39mexport with multiple collections
        �[32m✓ �[39mexport 1 of 2 collections
      .importJSON()
        positive
          �[32m✓ �[39mimport dump
        negative
          �[32m✓ �[39mshould not import if schema is different
    issues
      �[32m✓ �[39m#319 collections must be created before importDump
      �[32m✓ �[39m#1396 import/export should work with attachments

  database-lifecycle.ts
    �[32m✓ �[39mdo some writes updates and deletes and cleanups and reopens

  last.test.ts (dexie)
    �[32m✓ �[39mrun a minimal performance test to ensure the performance function works
    �[32m✓ �[39mensure all Memory RxStorage instances are closed
    �[32m✓ �[39mensure every db is cleaned up
    �[32m✓ �[39mensure all collections are closed
    �[32m✓ �[39mensure all BroadcastChannels are closed
    �[32m✓ �[39mensure all replication states are closed
    �[32m✓ �[39mensure all RemoteMessageChannels have been closed
    �[32m✓ �[39mensure all websockets have been closed
    �[32m✓ �[39mensure all leader electors are dead
    �[32m✓ �[39mexit the process

Chrome Headless 148.0.0.0 (Linux 0.0.0): Executed 1295 of 1295�[32m SUCCESS�[39m (56.296 secs / 55.511 secs)
�[32mTOTAL: 1295 SUCCESS�[39m



View full workflow run

Agent-Logs-Url: https://github.com/pubkey/rxdb/sessions/e5ba458c-8692-4a6e-9c4c-fc4c9a0c0251

Co-authored-by: pubkey <8926560+pubkey@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

@copilot ok, use promiseWait(20) no mather which rerunCount we have. still keep the rerunCount in place.

Done in 810fbd5. _execOverDatabase() now keeps rerunCount, but the retry yield is a fixed promiseWait(20) on every re-run instead of a growing backoff. I also updated the nearby regression wording to match that behavior. Validation: lint, build, the targeted rx-query regressions, and test:fast:memory passed; check-types still fails in the repo on pre-existing generated plugin import/export issues.

Copilot finished work on behalf of pubkey May 18, 2026 12:13
@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 18, 2026

ok works. now remove your test, we do not need that because it is way to complex to maintain or read. @copilot

Agent-Logs-Url: https://github.com/pubkey/rxdb/sessions/7f3e4920-b04a-47bf-9b2c-a369a840a9ee

Co-authored-by: pubkey <8926560+pubkey@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

ok works. now remove your test, we do not need that because it is way to complex to maintain or read. @copilot

Removed in 3248972. The extra #7067 regression test is gone, and the remaining rx-query async-storage coverage plus test:fast:memory still pass. check-types is still failing on the same pre-existing repo type issues.

Copilot finished work on behalf of pubkey May 18, 2026 12:22
@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 18, 2026

add to changelog with link to the issue #8444

@pubkey pubkey marked this pull request as ready for review May 18, 2026 13:39
@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 18, 2026

@copilot add to changelog with link to the issue #8444

Agent-Logs-Url: https://github.com/pubkey/rxdb/sessions/e10a28bf-0e68-41a2-ad64-b8fc905af251

Co-authored-by: pubkey <8926560+pubkey@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

add to changelog with link to the issue #8444

Added in e270637: orga/changelog/fix-live-query-rerun-race-8444.md now links issue #8444.

@copilot add to changelog with link to the issue #8444

Added in e270637: orga/changelog/fix-live-query-rerun-race-8444.md now links issue #8444.

Copilot finished work on behalf of pubkey May 18, 2026 13:44
@pubkey
Copy link
Copy Markdown
Owner

pubkey commented May 18, 2026

@copilot changelog is wrong. read the issue again. we just fixed the waiting times

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 18, 2026

@copilot changelog is wrong. read the issue again. we just fixed the waiting times

Corrected in aac3a2b. The changelog entry now describes the fixed waiting-time behavior under concurrent writes and links #8444.

Copilot finished work on behalf of pubkey May 18, 2026 13:53
@pubkey pubkey merged commit 254353d into master May 18, 2026
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants