-
-
Notifications
You must be signed in to change notification settings - Fork 15
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
plugin injection points #48
Comments
On my wishlist is an easier way to interface with the main methods so I don't have to decode arguments like LevelUP does. So I can interact with the API: Also, FTR, I'm open to level-hooks and possibly level-sublevel integration in core if necessary because I've come around to seeing what a useful abstraction it is and it could be made even more useful if it had better access in to core. |
+1 for level-sublevel and hooks in core, they are the sugar that makes this On 20 April 2013 03:11, Rod Vagg [email protected] wrote:
|
+1 for level-sublevel and hooks in core |
Okay, cool. @rvagg I agree, that would also make the implementation of sublevel much cleaner. In level hooks, I funnel these all together and call the hook with So, maybe, we could make each method just call another method: Sometimes, you want to turn a put/del into a batch - this is used in level-trigger (which is part of map-reduce) and level-master. Of course, if you add another op into a different range that needs to trigger hooks too. (adding it into the same range would probably make a infinite loop - this throws early in recent level-sublevel) Perhaps mutate could cb async? Or, it could be called in the context of a batch object? (overhead? - or pooled?) @rvagg did you intend the put/del/batch integration points to be async? Another thing that hooks does, is allow you to register a range for the hook. Hmm, I'm feeling like I need to do a write-up of exactly how level-sublevel works... |
I'd be way more likely to add sublevel support to |
It's like with node and core modules. The http module could live on npm too, but it's so core-y and heavily used that the core is the place it belongs. Since LevelUP is not only about accessing a LevelDB but soo much more about the idea of writing databases by layering abstractions through simple modules, extendability should be in core. And sublevel is there to help. |
I have mixed feelings about bundling stuff in to LevelUP, a lot of us think that Node core is too large as it is now and we don't want LevelUP getting too out of hand. At least we have LevelDOWN available as a perfectly usable and safe interface though. Re async hooks, I think that's a must and since the calls down to LevelDB are async anyway it's not really going to impact on the external API in any way. I did some basic benchmarking of the stuff I have on the branch I talked about in #92 with externr wired up and the impact was minimal even when you're funnelling everything through an additional layer of calls. V8 seems to be able to optimise effective noops away, so if you're not using anything in the plugin/hook layer then we ought to be able to get it to perform close to the current versinon. One thing I'd like to register while I'm thinking of it is that if we ended up bringing in sublevel to core (lets deal with hooks first, maybe sublevel after that), we have to make sure that if people don't want to use it then it doesn't get in the way. Currently it'll mangle your ranges so as to ignore anything Aside from that, I'm confident that @dominictarr has thought through this enough and discussed it enough on IRC to come up with a decent approach to start us off with hooks integration. Plus we have a good collection of LevelUP add-ons now to use as examples of the kinds of things we want to be able to do. Unfortunately I don't have a whole lot of time right now to spend reflecting on the best way to do this and not make a mess of it. |
we have |
Yeah, that's true, but we still need better hooks integration to make it all betterer. |
I also have reservations about pulling all of level-sublevel into levelup, Also whole heartedly agree that not using any fancy features should not affect the benchmarks! |
+1 to thin core So: var LevelUp = require('levelup');
var Sublevel = require('level-sublevel');
var MyPlugin = require('./my-plugin');
var db = LevelUp('/tmp/sublevel-example');
db.use(Sublevel);
db.use(MyPlugin);
db.put('key', ...);
// will first hit SubLevel
// then MyPlugin All plugins could be in the format: exports.put(key, value, next) { ... }
//next instead of callback as it's actually just moving down the plugin stack
exports.get(key, next) { ... }
// no 'del' implementation
// and then you could leave implementations out and missing ones would
// be inherited from the prior plugin (this would also mean the plugin in the
// middle of the stack could add a foo() function that the rise to the surface) Another idea: db.use(Sublevel, 'arg1', 'arg2');
// use could accept plugin constructor args Another idea: var db1 = db.extend(Sublevel, 'one');
var db2 = db.extend(Sublevel, 'two');
// could also extend instead of use, to keep the original db intact Another idea, incase its better to extend the class, rather than the instance: var LevelUp = require('levelup');
var Sublevel = LevelUp.extend(require('level-sublevel'));
//or
LevelUp.use(require('level-sublevel')); This may all be silly as I don't really know about how LevelUP/DOWN works internally... |
Also to address @dominictarr s points:
|
I already experimented with connect style middleware in leveled, see: https://github.com/juliangruber/node-leveled/tree/middleware#todo |
it basically has var req= {
key: key,
val: val,
method: method /* put, del, etc. */
}
var res = {
end: function (err, data) { /* finally save */ }
} |
Right, but instead passing all data through one function and saying |
how would you change the key for all operations? exports.put = function (key, value, next) {
key = 'foo'; // now you reassigned `key` instead of changing it
next();
} that would be fixed by passing an object: exports.put = function (kv, next) {
kv.key = 'foo';
next();
} this would be more performant than my api, I agree. But why just limit the performance increase to filtering out methods, we could also filter keys: function plugin (db) {
db.intersect({
method: 'put',
key: { start: 'aaa', end: 'ccc' },
/* or */
key: /[a-c]{3}/,
fn: function (kv, next) { /* ... */ }
})
} |
What about: exports.put = function (key, value, next) {
key = 'foo';
next(key, value);
} Where the params for next would mirror the args of the function, minus next |
Maybe a special case for error ? using:
Or maintain node style and:
|
this is roughly similar to what I have in #92 using externr; but I'm just not sure... tbh aside from not having the time to think this through clearly enough I'm really hoping that @dominictarr will come up with some brilliant abstraction that solves all our problems, but that might be a little unrealistic of me! |
@rvagg ah quite similar indeed. Though by going with the
As opposed to:
Also, in the former example, the monkey patching is only applied to one specific |
yeah, that's one thing I'd change about it now if I was to redo that pull request |
Could still be redone
😉 |
The point is that we're still searching for an acceptable abstraction here that serves the growing list of use-cases and gets past the need for the endless monkey-patching that we're all doing. I haven't redone that PR because I'm not overly committed to the approach (and neither was anyone else as it turned out!). |
Fair enough, I haven't been through all the use-cases yet and the connect-style of plugins may be inadequate, though when I find some time, I'll temper my idea with some research (only problem is the time finding >.<). Currently trying to make a case for leveldb at work... |
So, we have had quite a bit of discussion, and tried various approaches to implementing plugins in levelup
https://github.com/rvagg/node-levelup/issues/search?q=plugins
quick summation of what has happened so far, I started experimenting with a crude monkey patching based approach, but ran into trouble with handling ranges - each plugin had manage which ranges it affected, which was tricky. I later refactored this to create a subsection of the database, with level-sublevel. This is a great improvement, because it allows you to extend a range within leveldb as if it's a whole db.
@rvagg has also experimented with exposing various integration points into levelup, https://github.com/rvagg/node-levelup/issues/92
personally, I am highly in favor of combining these two, and even merging sublevel into levelup, or at least, adding integration points to levelup so that level-sublevel does not have to monkey patch it.
the question is: what is the list of integration points that we need?
** maybe. The ability to get the current values for keys before performing a mutation.
this would be useful for validation, and merging concurrent updates.
I have a plugin for this, but it hasn't been updated to work with level-sublevel yet. This differs from level-hooks, which only provides
a sync api.
A setup integration point will be useful for saving metadata about the database in the database, and maybe stuff like summaries about the current overall state - whether a schema change migration is complete, etc.
Any other suggestions?
The text was updated successfully, but these errors were encountered: