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

Merge levelup into abstract-leveldown #58

Closed
vweevers opened this issue Dec 30, 2018 · 11 comments
Closed

Merge levelup into abstract-leveldown #58

vweevers opened this issue Dec 30, 2018 · 11 comments
Labels
discussion Discussion

Comments

@vweevers
Copy link
Member

This is just an idea at this point, briefly discussed offline; we're not committing to anything. The premise: make any abstract-leveldown implementation a standalone, ready-to-go database.

Refactorings in the past years have increased modularity. It has allowed us to clean up various internals. That said, now that we have, there are downsides to the current architecture, with its many layers:

image

  • Adding custom behavior to e.g. level means having to peel off layers first, then add them back (most notably in subleveldown - that code is gnarly)
  • The layers share API surface with treacherously subtle differences (e.g. leveldown by itself returns Buffers by default, level(up) returns strings by default).
  • There are 4 forms of "encoding" now: encoding-down, serialization, asBuffer, and whatever the underlying storage does (e.g. IDB returning a Buffer as Uint8Array). Yet for an abstract-leveldown implementation there's no builtin primitive to decode/deserialize data.
  • Breaking changes in one layer typically bubble up, leading to what we call "the release dance". Prior to that, canary testing is often required because we can't foresee everything.
  • A rabbit hole of documentation and changelogs (level links to levelup links to encoding-down links to level-codec gives an example about leveldown links to level - you get the point)
  • Double allocation of callback functions, options objects, etc.
  • No builtin mechanism for asynchronous hooks (this could be achieved as another abstract-leveldown layer, but that doesn't make it available to levelup per se) (related: plugin extension points: merge level-hooks / level-sublevel #44).
  • Implementing manifests (Requirements for manifests #42) is hard.

Back to the idea: there are many open questions. Making a POC is probably the best way forward. To be clear: I don't want it to be a big batteries-included database. I think what we need are simpler primitives to enable composing (and maintaining!) such a database. Some "batteries" could be worth including, if they provide a foundation for userland modularity.

We have been recommending people to use levelup, and more so, level and friends, which is a clear indication that much of the functionality belongs in a single core component. Or to put it in other words, a single shell (the public API of abstract-leveldown) around a nut (the private API of abstract-leveldown).

@vweevers vweevers added the discussion Discussion label Dec 30, 2018
@ralphtheninja
Copy link
Member

ralphtheninja commented Dec 30, 2018

I'm all for simplifying. There's also an issue of performance. Maybe we could benchmark the hell out of level and compare with a benchmark done on a POC.

  • What would this mean for current implementations of abstract-leveldown?
  • Are we aiming for a smooth transition for implementations?
  • How would encodings fit in? (it must be easy to pull in e.g. bytewise and charwise).

Love the picture btw. This style is really nice. We should reuse it for documentation, blog post etc. How did you make it?

@vweevers
Copy link
Member Author

Love the picture btw. This style is really nice. We should reuse it for documentation, blog post etc. How did you make it?

Photoshop. I wanted to try its 3D features. And I'm never gonna do it again, lol. Took 10 crashes and retries and then 20 minutes waiting time just to render some damn plastic material 😄

@vweevers

This comment has been minimized.

@vweevers
Copy link
Member Author

vweevers commented Mar 30, 2019

Are we aiming for a smooth transition for implementations?

I say yes. The first version can and should be a drop-in replacement. To keep all the history and get regression tests for free.

Rough roadmap

The fork provides a starting point for later breaking changes, and a turning point for dependents: when they make the switch from abstract-leveldown to abstract-level, you no longer need levelup. Up until this point abstract-level is a drop-in replacement for abstract-leveldown that is also still safe to be wrapped by levelup - yet behaves the same as levelup if you don't. This is very optimistic, mind you. I'm sure there will be stumbling blocks along the way that may force us to reconsider this approach.

Let's make encodings "phase 2".

@vweevers

This comment has been minimized.

@vweevers

This comment has been minimized.

@vweevers

This comment has been minimized.

@vweevers
Copy link
Member Author

💡 If abstract-leveldown implementations are encoding-aware then leveldown could implement the IndexedDB comparator (which sorts almost the same as bytewise/charwise) natively.

@vweevers
Copy link
Member Author

vweevers commented Jan 2, 2021

Calling this abstract-level going forward. It ain't down, it ain't up, it's level.

@vweevers
Copy link
Member Author

vweevers commented Jan 2, 2021

Let's make encodings "phase 2".

I think it will be easier to do this the right way from the get-go. I want to support encodings out of the box, replacing both _serialize() (which is just another encoding API with a different name) and asBuffer (which in a way expresses a relation between codecs, for example that json is also utf8). I'm working out the details.

vweevers added a commit to Level/errors that referenced this issue Sep 18, 2021
So that when `abstract-leveldown` starts using `level-errors`, and
such a db is wrapped with `levelup`, `levelup` will not rewrap
errors and lose stack trace information in the process. I.e.:

```
// Error created by abstract-leveldown
const err = new ReadError('example')

// Error created by levelup
const wrapped = new ReadError(err)

assert(wrapped === err)
```

Ref Level/community#58
vweevers added a commit to Level/errors that referenced this issue Sep 24, 2021
So that when `abstract-leveldown` starts using `level-errors`, and
such a db is wrapped with `levelup`, `levelup` will not rewrap
errors and lose stack trace information in the process. I.e.:

```
// Error created by abstract-leveldown
const err = new ReadError('example')

// Error created by levelup
const wrapped = new ReadError(err)

assert(wrapped === err)
```

Ref Level/community#58
vweevers added a commit to Level/abstract-leveldown that referenced this issue Oct 3, 2021
Very similar to `levelup` but more precise. If `open()` and
`close()` are called repeatedly (while the previous call has not
yet completed) the last call dictates the final status. Callbacks
are not called until any pending state changes are done, meaning
that the status is not 'opening' or 'closing'. Same for events.

For example, in a sequence of calls like `open(); close(); open()`
the final status will be 'open', only the second call will error,
only an 'open' event is emitted, and all callbacks will see that
status is 'open'. The callbacks are called in the order that the
`open()` or `close()` calls were made.

In addition, unlike on `levelup`, it is safe to call `open()` while
status is 'closing'. It will wait for closing to complete and then
reopen.

We should now have complete safety, including in `leveldown` because
the native code there delays `close()` if any operations are in
flight. In other words, the JavaScript side in `abstract-leveldown`
prevents new operations before opening the db, and the C++ side in
`leveldown` prevents closing the db before operations completed.

Ref Level/leveldown#8
Ref Level/community#58
@vweevers
Copy link
Member Author

vweevers commented Nov 9, 2021

This thread got messy with my frequent edits and inline task lists. Continuing at https://github.com/Level/abstract-level.

@vweevers vweevers closed this as completed Nov 9, 2021
vweevers added a commit to Level/abstract-level that referenced this issue Jan 2, 2022
Very similar to `levelup` but more precise. If `open()` and
`close()` are called repeatedly (while the previous call has not
yet completed) the last call dictates the final status. Callbacks
are not called until any pending state changes are done, meaning
that the status is not 'opening' or 'closing'. Same for events.

For example, in a sequence of calls like `open(); close(); open()`
the final status will be 'open', only the second call will error,
only an 'open' event is emitted, and all callbacks will see that
status is 'open'. The callbacks are called in the order that the
`open()` or `close()` calls were made.

In addition, unlike on `levelup`, it is safe to call `open()` while
status is 'closing'. It will wait for closing to complete and then
reopen.

We should now have complete safety, including in `leveldown` because
the native code there delays `close()` if any operations are in
flight. In other words, the JavaScript side in `abstract-leveldown`
prevents new operations before opening the db, and the C++ side in
`leveldown` prevents closing the db before operations completed.

Ref Level/leveldown#8
Ref Level/community#58
@vweevers vweevers unpinned this issue Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion
Projects
None yet
Development

No branches or pull requests

2 participants