Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Require deferredOpen support #84

Closed
vweevers opened this issue Oct 20, 2019 · 1 comment · Fixed by #89
Closed

Require deferredOpen support #84

vweevers opened this issue Oct 20, 2019 · 1 comment · Fixed by #89
Labels
discussion Discussion semver-major Changes that break backward compatibility

Comments

@vweevers
Copy link
Member

vweevers commented Oct 20, 2019

To simplify the open/close handling on sublevels (and fix issues like #60) I propose the following.

  1. Only accept a db that supports deferredOpen (currently that's only levelup, in the future also abstract-leveldown). In subleveldown we then never open or close the parent db. More precisely: we never initiate a state change. The parent db must open itself (or once closed by the user, be explicitly reopened by the user) because sublevels shouldn't touch (the state of) the rest of the db.
  2. We can then have subdown._open() wait for an open event of the parent db. It would be nice to not have an _open() method at all, i.e. sublevels not having to care about whether the db is open, because deferredOpen support means you can always do operations on it, but this currently can't work because we operate on an unwrapped db. When abstract-leveldown gets deferredOpen support, things will get better.
@vweevers vweevers added the discussion Discussion label Oct 20, 2019
@vweevers vweevers added this to Backlog in Level (old board) via automation Oct 20, 2019
@vweevers vweevers moved this from Backlog to To Do in Level (old board) Oct 20, 2019
@vweevers vweevers added the semver-major Changes that break backward compatibility label Oct 20, 2019
@vweevers vweevers changed the title Require deferredOpen support and remove open hook Require deferredOpen support Oct 20, 2019
@vweevers
Copy link
Member Author

vweevers commented Oct 20, 2019

The following cases will become illegal.

Creating a sublevel on a closed db

db.close(function (err) {
  subdb(db, 'example').on('error', function (err) {
    throw err // Error: Parent database is not open
  })
})

Creating a sublevel on a closing db / Closing a db while sublevel is opening

subdb(db, 'example').on('error', (err) => {
  throw err // Error: Parent database is not open
})

db.close(function () {})

@vweevers vweevers moved this from To Do to In progress in Level (old board) Apr 4, 2020
vweevers added a commit that referenced this issue Apr 4, 2020
DeferredOpen means that the db opens itself and defers operations
until it's open. Currently that's only supported by level(up) and
friends. Before, subleveldown would also accept abstract-leveldown
db's that were not wrapped in levelup.

Opening and closing a sublevel no longer opens or closes the parent
db. The sublevel does wait for the parent to open (which in the
case of levelup already happens automatically) but never initiates
a state change.

Drops support of old modules:

- memdb (use level-mem instead)
- deferred-leveldown < 2.0.0 (and thus levelup < 2.0.0)
- abstract-leveldown < 2.4.0

Closes #84, #83 and #60.
vweevers added a commit that referenced this issue Apr 4, 2020
DeferredOpen means that the db opens itself and defers operations
until it's open. Currently that's only supported by level(up) and
friends. Before, subleveldown would also accept abstract-leveldown
db's that were not wrapped in levelup.

Opening and closing a sublevel no longer opens or closes the parent
db. The sublevel does wait for the parent to open (which in the
case of levelup already happens automatically) but never initiates
a state change.

Drops support of old modules:

- memdb (use level-mem instead)
- deferred-leveldown < 2.0.0 (and thus levelup < 2.0.0)
- abstract-leveldown < 2.4.0

Closes #84, #83 and #60.
@vweevers vweevers moved this from In progress to Review in Level (old board) Apr 4, 2020
vweevers added a commit that referenced this issue Apr 5, 2020
DeferredOpen means that the db opens itself and defers operations
until it's open. Currently that's only supported by levelup (and
levelup factories like level). Previously, subleveldown would also
accept abstract-leveldown db's that were not wrapped in levelup.

Opening and closing a sublevel no longer opens or closes the parent
db. The sublevel does wait for the parent to open (which in the
case of levelup already happens automatically) but never initiates
a state change.

If one closes the parent but not the sublevel, subsequent
operations (like get and put) on the sublevel will yield an error,
to prevent segmentation faults from underlying stores.

Drops support of old modules:

- memdb (use level-mem instead)
- deferred-leveldown < 2.0.0 (and thus levelup < 2.0.0)
- abstract-leveldown < 2.4.0

Closes #84, #83 and #60.
Level (old board) automation moved this from Review to Done Apr 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion semver-major Changes that break backward compatibility
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

1 participant