fix: Race condition for pouchdb hanging indefinitely when multiple instances are used#8513
Merged
garethbowen merged 4 commits intoapache:masterfrom Dec 8, 2022
Conversation
2 tasks
This is a simple repro for a problem discovered in PouchDB 7.3.0. This was originally discovered in RxDB. Discussion here: pubkey/rxdb#3807. ``` 134 passing (2s) 3 pending 1 failing 1) test.memory-adapter.js Race condition initially discovered with PouchDB in-memory-adapter 7.3.0: Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. at listOnTimeout (node:internal/timers:559:17) at processTimers (node:internal/timers:502:7) ```
PaulMest
force-pushed
the
repro/race-condition-with-memory-adapter
branch
from
dfd0e49 to
d04021f
Compare
Contributor
Author
|
I've updated my PR:
Hope this gets included into a future release! 🤞🏼 |
PaulMest
changed the title
garethbowen
requested changes
Dec 7, 2022
Contributor
garethbowen
left a comment
garethbowen
left a comment
There was a problem hiding this comment.
Thanks for the test, that's really helpful. I've added a suggestion.
| var adapterName = functionName(leveldown); | ||
| var adapterStore = dbStores.get(adapterName); | ||
| var keys = [...adapterStore.keys()].filter(k => k.includes("-mrview-")); | ||
| var keys = [...adapterStore.keys()].filter(k => k.includes(name)).filter(k => k.includes("-mrview-")); |
Contributor
There was a problem hiding this comment.
I think there's a small issue here if you have two dbs with the same prefix, for example, "test-db" and "test-db-2". If you close "test-db" the same race condition will occur. I think we can make the matching string more restrictive to avoid these edge cases, eg:
Suggested change
| var keys = [...adapterStore.keys()].filter(k => k.includes(name)).filter(k => k.includes("-mrview-")); | |
| var viewNamePrefix = PouchDB.prefix + name + "-mrview-"; | |
| var keys = [...adapterStore.keys()].filter(k => k.includes(viewNamePrefix)); |
You'll have to import PouchDB too, and I haven't actually run this code, but I think it'll work.
Contributor
Author
There was a problem hiding this comment.
Thanks for looking this over! Ran this locally and it seemed to work. Updated the tests and used the code snippet you provided. Just pushed the commit. CI should complete in about 10 minutes to verify.
maximerety
added a commit
to TankerHQ/pouchdb
that referenced
this pull request
Dec 13, 2022
…ly when multiple instances are used" This reverts commit 24157fc.
CaptainAchab
pushed a commit
to TankerHQ/sdk-js
that referenced
this pull request
Jan 3, 2023
Release notes: https://github.com/pouchdb/pouchdb/releases/tag/8.0.0 It includes a fix for our issue with multiple PouchDB instances used concurrently, see: apache/pouchdb#8513. Also note that we are waiting for a clarification about the way dependent databases are handled in PouchDB. This could lead to new changes, see: apache/pouchdb#8574. We still allow compat with versions >= 7.2.2 < 7.3.0 for ease of use alongside previous Tanker SDK versions that required these versions. We ban the 7.3.x versions though since they are affected by the aforementioned bug.
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
2 participants
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.
This is a self-contained repro for a problem discovered in PouchDB 7.3.0 where 2 databases using the
memoryadapter could impact each other and ultimately become non-responsive.This was originally discovered in the RxDB test suite. Discussion here: pubkey/rxdb#3807.