Skip to content
This repository has been archived by the owner on Dec 2, 2024. It is now read-only.

Create iterators in order #75

Merged
merged 2 commits into from
Sep 17, 2019
Merged

Create iterators in order #75

merged 2 commits into from
Sep 17, 2019

Conversation

vweevers
Copy link
Member

This fixes the (currently disabled) create-stream-vs-put-racecondition.js test in levelup (Level/levelup#497). That test was originally meant for leveldown I believe, before it had snapshots. Today, it's more relevant for memdown, which is secretly synchronous. E.g. it does (simplified):

memdown.prototype.put = function (.., callback) {
  this.store.put(..)
  process.nextTick(callback)
}

Rather than:

memdown.prototype.put = function (.., callback) {
  process.nextTick(function () {
    this.store.put(..)
    callback()
  })
}

Before this PR, if you did the following on a deferred db:

db.iterator()
db.put(..)

Then on the underlying db it would actually be executed as if you did:

db.put(..)
db.iterator()

Which in the case of memdown, because it is synchronous, means the put was written before the iterator (snapshot) is created. Does it matter? Probably not a whole lot, but I think this PR makes for a less surprising behavior.

@vweevers vweevers added the semver-patch Bug fixes that are backward compatible label Sep 15, 2019
@@ -8,7 +8,6 @@ function DeferredLevelDOWN (db) {
AbstractLevelDOWN.call(this, '')
this._db = db
this._operations = []
this._iterators = []
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, damnit, this would affect reachdown 😄 (currently detects deferred-leveldown by presence of _operations and _iterators)

Copy link
Member Author

@vweevers vweevers Sep 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interest of moving on, I'll restore this._iterators = [], with a comment saying it's temporary.

Edit: or, adding type (Level/community#82) might do the trick.

Because now that db._iterators is gone, reachdown can no longer
feature-detect deferred-leveldown.
@vweevers vweevers merged commit 1ba5307 into master Sep 17, 2019
@vweevers vweevers deleted the create-iterators-in-order branch September 17, 2019 18:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
semver-patch Bug fixes that are backward compatible
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants