Skip to content

Conversation

@achingbrain
Copy link
Member

If we try to shut a node down while a query is still in flight it errors out. The change here is to retry deleting the databse when we detect that it's still open, 5x attempts, each waiting 1-5s before trying again. If it's still open after that, bail.

If we try to shut a node down while a query is still in flight it
errors out.  The change here is to retry deleting the databse when
we detect that it's still open, 5x attempts, each waiting 1-5s before
trying again.  If it's still open after that, bail.
@achingbrain
Copy link
Member Author

I think this is necessary because close() on IndexedDB is a lie:

The close() method, when invoked, must run these steps:

Run close a database connection with this connection.

The connection will not actually close until all outstanding transactions have completed. Subsequent calls to close() will have no effect.

https://w3c.github.io/IndexedDB/#dom-idbdatabase-close

@achingbrain achingbrain requested a review from hugomrdias May 13, 2020 11:55
@hugomrdias
Copy link
Member

its not enought to wait on success and remove the onblocked?

@achingbrain
Copy link
Member Author

I don't know, I assumed the onblocked handler there was for a reason?

@hugomrdias
Copy link
Member

hugomrdias commented May 13, 2020

looking into some libs this hook seems to be used for debug and is mostly optional.
when i added the onblocked i think was more concerned with getting all the info possible than anything else. can you run the tests without it and see if it fails ?

@achingbrain
Copy link
Member Author

It seems happy enough; the tests pass so I've pulled out the retry and just wait for the onsuccess.

@hugomrdias hugomrdias changed the title fix: retry deleting database fix: remove onBlocked hook from delete database May 13, 2020
@hugomrdias hugomrdias merged commit e4d6f80 into master May 13, 2020
@hugomrdias hugomrdias deleted the fix/retry-closing-database branch May 13, 2020 16:44
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.

3 participants