hooks #29

Closed
Raynos opened this Issue Nov 20, 2012 · 27 comments

Comments

Projects
None yet
5 participants
@Raynos
Member

Raynos commented Nov 20, 2012

Hooks on put and del that allow you to turn put and del into a batch.

If your usage of levelup is complicated you may want to intercept del and put and also store a marker in the database saying "some kind of job needs to run"

@dominictarr needs this for map-reduce.

The main reason for this is that you cant use before:put or after:put since if the process crashes then either you have the data and no marker or the marker and no data in the database.

You want to convert the put or del into an atomic batch operation.

Raynos added a commit to Raynos/node-levelup that referenced this issue Nov 20, 2012

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

We need to figure out a good pattern for extending levelup,
but we don't know how many plugins we'll need.

So far I have queue, and map-reduce.
I can also I can imagine a few more...
something for writelocks, and some modules for different versioning approaches.

So far, I'm emitting namespaced events, and adding methods to the db object.

Maybe there will be 10-20 modules... this isn't jQuery.

Contributor

dominictarr commented Nov 20, 2012

We need to figure out a good pattern for extending levelup,
but we don't know how many plugins we'll need.

So far I have queue, and map-reduce.
I can also I can imagine a few more...
something for writelocks, and some modules for different versioning approaches.

So far, I'm emitting namespaced events, and adding methods to the db object.

Maybe there will be 10-20 modules... this isn't jQuery.

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

or, now that I said that, will it become jQuery?

(I better put this other comment to prevent this becoming jQuery)

Contributor

dominictarr commented Nov 20, 2012

or, now that I said that, will it become jQuery?

(I better put this other comment to prevent this becoming jQuery)

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

If we start adding convenience things like deleteRange, then the number of modules could get pretty big.

Contributor

dominictarr commented Nov 20, 2012

If we start adding convenience things like deleteRange, then the number of modules could get pretty big.

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

@dominictarr map-reduce and queue are not plugins

They are modules that get a leveldb instance passed to them as a function

var db = levelup(...)

mapReduce(db, ...)

A module should not add methods to db it should return a new seperate object.
A module should not emit stuff on db, it should return an event emitter.

We should however put stuff in levelup that requires calls to C++ for efficiency or simply needed to be able to cleanly write functions that take db objects as parameters and don't monkey patch or mutate the db.

Member

Raynos commented Nov 20, 2012

@dominictarr map-reduce and queue are not plugins

They are modules that get a leveldb instance passed to them as a function

var db = levelup(...)

mapReduce(db, ...)

A module should not add methods to db it should return a new seperate object.
A module should not emit stuff on db, it should return an event emitter.

We should however put stuff in levelup that requires calls to C++ for efficiency or simply needed to be able to cleanly write functions that take db objects as parameters and don't monkey patch or mutate the db.

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

yeah, but what if we are adding modules that need each other?
like map-reduce needs queue,
and the data in the database is already a single global scope...
so I feel like reflecting that in the user api is actually acceptable.

Contributor

dominictarr commented Nov 20, 2012

yeah, but what if we are adding modules that need each other?
like map-reduce needs queue,
and the data in the database is already a single global scope...
so I feel like reflecting that in the user api is actually acceptable.

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

I'm going to write a module that does live querying

var stream = query(db, { start: ... , end: ... })

// stream never ends and gets new values when put and delete fire.
// like batch but a readable stream
stream.pipe(WriteStream(function (chunk) {
    chunk.type; // put or delete
    chunk.value;
    chunk.key;
}))
Member

Raynos commented Nov 20, 2012

I'm going to write a module that does live querying

var stream = query(db, { start: ... , end: ... })

// stream never ends and gets new values when put and delete fire.
// like batch but a readable stream
stream.pipe(WriteStream(function (chunk) {
    chunk.type; // put or delete
    chunk.value;
    chunk.key;
}))
@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

@dominictarr

map-reduce needs queue

thats what peer dependencies are for

@domenic peer dependencies man! What can I do to land them.

Member

Raynos commented Nov 20, 2012

@dominictarr

map-reduce needs queue

thats what peer dependencies are for

@domenic peer dependencies man! What can I do to land them.

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

what are peer dependencies?

Contributor

dominictarr commented Nov 20, 2012

what are peer dependencies?

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member
@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

@Raynos your stream query is a much simpler case, because it doesn't need to put anything in the database, or change it's behavior.

Contributor

dominictarr commented Nov 20, 2012

@Raynos your stream query is a much simpler case, because it doesn't need to put anything in the database, or change it's behavior.

@domenic

This comment has been minimized.

Show comment Hide comment
@domenic

domenic Nov 20, 2012

@Raynos at this point we're just waiting for @isaacs to get back from his world tour and review some patches!

domenic commented Nov 20, 2012

@Raynos at this point we're just waiting for @isaacs to get back from his world tour and review some patches!

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

@domenic do it the proper way and make a public PR on npm

Member

Raynos commented Nov 20, 2012

@domenic do it the proper way and make a public PR on npm

@domenic

This comment has been minimized.

Show comment Hide comment
@domenic

domenic Nov 20, 2012

@Raynos that needs to wait for the public PR on read-installed to be accepted.

domenic commented Nov 20, 2012

@Raynos that needs to wait for the public PR on read-installed to be accepted.

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

@domenic badassery! @isaacs I want this.

Member

Raynos commented Nov 20, 2012

@domenic badassery! @isaacs I want this.

@rvagg

This comment has been minimized.

Show comment Hide comment
@rvagg

rvagg Nov 20, 2012

Member

I wonder.. if you think you might need to extend all over the place, perhaps it might be better to approach this through events? I'm not sure this covers the use-cases you have but something like this:

var Stoppable = { stop: function () { this.stopped = true } }
// ...
// inside put():
var stoppable = Object.create(Stoppable)
this.emit('put:before', key, value, stoppable)
if (stoppable.stopped !== true) {
  // do the put operation
}
// ...
// inside client code:
levelup.on('put:before', function (key, value, stoppable) {
  if (/^derp/.test(key)) stoppable.stop()
})

Then you can prevent default behaviours where you need to, tho you do have the overhead of an Object.create each time you emit one of the stoppable events.

We could even replace the key & value arguments with a full DOM-style Event object that has them as properties along with the equivilant of Event#preventDefault().

Member

rvagg commented Nov 20, 2012

I wonder.. if you think you might need to extend all over the place, perhaps it might be better to approach this through events? I'm not sure this covers the use-cases you have but something like this:

var Stoppable = { stop: function () { this.stopped = true } }
// ...
// inside put():
var stoppable = Object.create(Stoppable)
this.emit('put:before', key, value, stoppable)
if (stoppable.stopped !== true) {
  // do the put operation
}
// ...
// inside client code:
levelup.on('put:before', function (key, value, stoppable) {
  if (/^derp/.test(key)) stoppable.stop()
})

Then you can prevent default behaviours where you need to, tho you do have the overhead of an Object.create each time you emit one of the stoppable events.

We could even replace the key & value arguments with a full DOM-style Event object that has them as properties along with the equivilant of Event#preventDefault().

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

The overhead of Object.create can be avoided by using new Stoppable.

However I'm not sure whether this is a better api then the hook suggestion.

At the very least hooks should have a detection for whether the array was mutated at all and if not just do the normal cheaper operation.

Member

Raynos commented Nov 20, 2012

The overhead of Object.create can be avoided by using new Stoppable.

However I'm not sure whether this is a better api then the hook suggestion.

At the very least hooks should have a detection for whether the array was mutated at all and if not just do the normal cheaper operation.

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

@dominictarr @ricardobeat tie break the api bikesheddery please.

Member

Raynos commented Nov 20, 2012

@dominictarr @ricardobeat tie break the api bikesheddery please.

@rvagg

This comment has been minimized.

Show comment Hide comment
@rvagg

rvagg Nov 20, 2012

Member

The resort to batch() makes it look very specific to a particular use-case and perhaps not as helpful for other hook use-cases. Shouldn't you use the hook to decide if you need to halt the current put() operation and resort to a batch() external to LevelUP?

Member

rvagg commented Nov 20, 2012

The resort to batch() makes it look very specific to a particular use-case and perhaps not as helpful for other hook use-cases. Shouldn't you use the hook to decide if you need to halt the current put() operation and resort to a batch() external to LevelUP?

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

@rvagg I can't think of any other use case for a hook.

but your right it should probably be less specific.

What about return false instead of stoppable.

Member

Raynos commented Nov 20, 2012

@rvagg I can't think of any other use case for a hook.

but your right it should probably be less specific.

What about return false instead of stoppable.

@rvagg

This comment has been minimized.

Show comment Hide comment
@rvagg

rvagg Nov 20, 2012

Member

makes sense

Member

rvagg commented Nov 20, 2012

makes sense

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

return false does require a .hook(put, cb) api since emit doesn't return a useful value.

Member

Raynos commented Nov 20, 2012

return false does require a .hook(put, cb) api since emit doesn't return a useful value.

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

Actually the way hook works is nice for the batch use case because two modules can both patch the array and add their changes.

Where as your use-case would have two modules both cancel the put and both create a new patch of put + their own thing. This would be a bug.

hook is specifically for the turn operation into batch use-case where multiple hooks can share the same batch

Member

Raynos commented Nov 20, 2012

Actually the way hook works is nice for the batch use case because two modules can both patch the array and add their changes.

Where as your use-case would have two modules both cancel the put and both create a new patch of put + their own thing. This would be a bug.

hook is specifically for the turn operation into batch use-case where multiple hooks can share the same batch

@rvagg

This comment has been minimized.

Show comment Hide comment
@rvagg

rvagg Nov 20, 2012

Member

well, whatever you go with, maybe just make sure it makes sense by actually putting it into use first.

Member

rvagg commented Nov 20, 2012

well, whatever you go with, maybe just make sure it makes sense by actually putting it into use first.

@Raynos

This comment has been minimized.

Show comment Hide comment
@Raynos

Raynos Nov 20, 2012

Member

Ok so this should just be an npm module. Too much debating

Member

Raynos commented Nov 20, 2012

Ok so this should just be an npm module. Too much debating

@juliangruber

This comment has been minimized.

Show comment Hide comment
@juliangruber

juliangruber Nov 20, 2012

Member

I've written a middleware implementation for leveled, similar to connect. Check out the middleware branch and its example middleware

Member

juliangruber commented Nov 20, 2012

I've written a middleware implementation for leveled, similar to connect. Check out the middleware branch and its example middleware

@juliangruber

This comment has been minimized.

Show comment Hide comment
@juliangruber

juliangruber Nov 20, 2012

Member

And for adding methods, do it c-style

reduce(db, your_args)
// or
var reduce = require('reduce').bind(this, db)
reduce(your_args)

Reduce can pass its db reference to any child modules that need the db.

Member

juliangruber commented Nov 20, 2012

And for adding methods, do it c-style

reduce(db, your_args)
// or
var reduce = require('reduce').bind(this, db)
reduce(your_args)

Reduce can pass its db reference to any child modules that need the db.

@dominictarr

This comment has been minimized.

Show comment Hide comment
@dominictarr

dominictarr Nov 20, 2012

Contributor

This discussion is silly. we can't reason a prior i what we'll need, I am just gonna play around with this until I have something working then tidy it up, refactor it out and make it a pull request or separate module.

I do have a few ideas for things that will need hooks though, but building them will probably reveal issues that I havn't thought of yet.

One thing that will require a hook is inserting extra docs on a put.
that requires turning a single put or delete into a batch. This would be useful for queuing up jobs on an insert,
map reduce, or to implement message fanout on a twitter clone, and also for writing logs for scuttlebutt style replication.

This first sort of hook could by synchronous.

If we had async hooks, we could implement write locks, this could be used to enforce a versioning method on documents. you'd intercept a put, and retrive the document, and then use a custom merge function to decide if that put is a valid update to that document.

Contributor

dominictarr commented Nov 20, 2012

This discussion is silly. we can't reason a prior i what we'll need, I am just gonna play around with this until I have something working then tidy it up, refactor it out and make it a pull request or separate module.

I do have a few ideas for things that will need hooks though, but building them will probably reveal issues that I havn't thought of yet.

One thing that will require a hook is inserting extra docs on a put.
that requires turning a single put or delete into a batch. This would be useful for queuing up jobs on an insert,
map reduce, or to implement message fanout on a twitter clone, and also for writing logs for scuttlebutt style replication.

This first sort of hook could by synchronous.

If we had async hooks, we could implement write locks, this could be used to enforce a versioning method on documents. you'd intercept a put, and retrive the document, and then use a custom merge function to decide if that put is a valid update to that document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment