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

Enable abstract-leveldown stores to emit events for levelup #321

Closed
u8sand opened this issue Dec 26, 2018 · 13 comments
Closed

Enable abstract-leveldown stores to emit events for levelup #321

u8sand opened this issue Dec 26, 2018 · 13 comments
Labels
discussion Discussion enhancement New feature or request

Comments

@u8sand
Copy link

u8sand commented Dec 26, 2018

In some cases, an underlying store may change via some other external event, it would be nice if it were possible to capture this and utilize it in the levelup event emitter.

Example: browser localStorage can be updated on a different browser window; if we catch it we could potentially trigger a levelup.emit('put', key, val).

@vweevers
Copy link
Member

Current solution is to either:

  1. Get the underlying store via db.db (if db is open, where db is a levelup instance) or db.db.db (if db is opening), then attach your event listeners
  2. Get the underlying store via db._db (note this is a private, undocumented property of levelup)

We could also consider to:

  1. Forward all events (an abstract-leveldown implementation would have to expose its event names somehow) (and I think, to justify such a feature, we need more use cases).
  2. Forward a predefined list of events (e.g. put, del, ..)

Lastly, there are a couple of ideas floating around that could help:

  1. Get the underlying store via a proposed db.down() method (some discussion in Current functionality and the future (hoping on support for all use cases) subleveldown#34 (comment))
  2. Merge levelup functionality into abstract-leveldown (very, very early stages, only discussed offline so far) so that an abstract-leveldown implementation becomes a standalone, ready-to-go db.

@vweevers vweevers added the enhancement New feature or request label Dec 27, 2018
@u8sand
Copy link
Author

u8sand commented Dec 27, 2018

The workaround I've considered is to expose an emitter and then override levelup's constructor to pass the emitter through the levelup store. I like the idea of your last (2) given the pretty large amount of overlap between the two.

Please let me know if there is anything I can do to move these discussions forward or contribute. I've recently made this, and built in my workaround for supporting a changes event-emitter (https://github.com/u8sand/easier-abstract-leveldown/blob/master/src/abstract.ts#L55-L56 for reference)

@vweevers
Copy link
Member

Are there backends other than webstorage for which this feature is useful? If not, the currently available workarounds are good enough for now IMO (that's not to dismiss the use case - it's a matter of priority).

@u8sand
Copy link
Author

u8sand commented Dec 27, 2018

I'm planning on building a number of stores based on remote WebAPIs that support changes which is why I even brought this up. Though as you said, I can continue working around the issue, I think its something the entire level community could take advantage of.

Another good example is CouchDB which emits a changes feed. In general, even a file can be changed while an application is running; I think if it is possible to capture these changes it should be captured for applications to be capable of ever noticing any of those changes and refresh their displays.

Other examples I've already come across include Google Drive's API. In general, Abstract LevelDOWN is a bottleneck making it so that my application cannot respond to changes in the remote stores, while levelup essentially has capacity for the feature already..

I'll clarify--I need the levelup instance from the leveldown one, not the other way around as was suggested at the beginning

@vweevers
Copy link
Member

I'm planning on building a number of stores based on remote WebAPIs that support changes which is why I even brought this up.

Interesting!

I think if it is possible to capture these changes it should be captured for applications to be capable of ever noticing any of those changes and refresh their displays.

Could you rephrase? Not sure I understand.

@u8sand
Copy link
Author

u8sand commented Dec 27, 2018

Google Drive documents can be modified at any time, suppose we're writing key/values to a database which is built out of a google drive document--say just a JSON text file, location refers to the name of the file. We can do batch writes and reads to that file and eventually flush it to the remote store (google drive).

But because it's google drive, that file may have changed since the last time we looked at it, effectively causing some batch put/delete to our store (not by us directly, so we would not have called "put" or "del" explicitly)--if we can emit those events, our application will be aware of how our underlying store has mutated. It can then update whatever information used to be in it if keys were overwritten or deleted, etc..

The alternative to this is to constantly sync to an in-memory store of some kind, which may not be desirable if, for instance it's too big to fit in memory. We can use things like cachedown to speed up these potentially slow remote dbs.

@vweevers
Copy link
Member

I'll clarify--I need the levelup instance from the leveldown one, not the other way around as was suggested at the beginning

It'd be weird for an abstract-leveldown implementation to have knowledge of levelup. So I'll clarify too (I want to make sure we're on the same page): I meant that your application can reach down into levelup to get the underlying db.

Then it's also up to your application (or some wrapper module) to decide how to deal with changes emitted by the underlying store. Because as it stands, the change events emitted by levelup have a specific meaning: "something was changed through levelup".

Changing that meaning will require some thought. E.g. could it affect modules in the ecosystem (like live streams and indexing modules) that rely on these events.

@u8sand
Copy link
Author

u8sand commented Dec 27, 2018

I see; in that case you're right-- for now I can extend my LevelDOWNs to behave as emitters, and join the emitters at the application level through the levelup.db.

But I hope this discussion can continue to consider this feature being an added part to abstract leveldown. As mentioned, there are certainly at least a few stores which could be extended to take advantage of this already.

@vweevers
Copy link
Member

But I hope this discussion can continue to consider this feature being an added part to abstract leveldown. As mentioned, there are certainly at least a few stores which could be extended to take advantage of this already.

Yes!

@vweevers vweevers added the discussion Discussion label Dec 27, 2018
@MeirionHughes
Copy link
Member

MeirionHughes commented Jun 22, 2019

This could (potentially) lead to some leaks if you create subleveldown instances on the fly, if they subscribe to the base db, which holds the instance reference indefinitely, and then you forget to explicitly close the subdb instance.

Currently I don't see anything that doesn't stop the gc can just discarding subleveldown instances if you throw the reference away.

@vweevers
Copy link
Member

This could (potentially) lead to some leaks if you create subleveldown instances on the fly, if they subscribe to the base db, which holds the instance reference indefinitely, and then you forget to explicitly close the subdb instance.

That's a good point. Perhaps we could lazily subscribe to events. But the better course - altogether - is to get rid of levelup.

@MeirionHughes
Copy link
Member

Been looking at this again - As I need to implement "down-down" replication but then bubble up the events up through into the high-level 'ups' and 'subs' (4-way binding). Not an easy change/fix, especially when using sublevels.

PouchDB uses event emitters on its "Nut" (abstract-leveldown like) and they have commits that fix emitter leaks. So that might be a good source on how to go about doing it if there is some will to add support into abstract-leveldown

@vweevers vweevers added this to Level Dec 4, 2021
@vweevers vweevers moved this to Backlog in Level Dec 4, 2021
@vweevers vweevers moved this from Backlog to Todo in Level Dec 29, 2021
@vweevers
Copy link
Member

But the better course - altogether - is to get rid of levelup.

https://github.com/Level/abstract-level

Repository owner moved this from Todo to Done in Level Jan 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
discussion Discussion enhancement New feature or request
Projects
Status: Done
Development

No branches or pull requests

3 participants