-
-
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
My approach to plugins #51
Comments
Why all this complex machinery when you can call |
Its good to see a discussion like this to allow plugins architecture to be performant. Is it possible that the advantages and disadvantages of the JS versus c++ approach be listed to provide more clarity ? For my own usage i favour a pure JS solution to allow easy of developing plugins. |
@Raynos I'm trying to come up with an approach that makes a more consistent plugin ecosystem so that users don't have to do something new & different for each thing they want to bolt on. I know you're comfortable plugging many tiny bits together to form a complete, customised whole but I'd like to make this as easy as possible for the average dev who doesn't have time to dig into the tiny details of each element they need in order to come up with a solution that fits their problem. I'm only using @Raynos can you clarify your question about C++ extensions and global pollution? @gedw99 pure JS implementations are fine for most things that could conceivably extend LevelUP but there are some cases where that's not possible (e.g. comparators that adjust internal sorting in LevelDB) and other cases where the efficiency gain is significant, such as |
If anyone's interested in the speed impact of doing a native
|
massive difference. i guess then we have to have some plugins at c++ level. so for the c+ G On 8 March 2013 14:45, Rod Vagg [email protected] wrote:
Contact details: |
Well @gedw99, they wouldn't be enabled or even available by default. So far nobody is suggesting they actually get bundled with the core. So you'd have to explicitly install them with npm and either invoke them with their own custom magic (@Raynos' approach) or inject them with special, consistent LevelUP magic (my approach). I keep on thinking up awesome native plugins now that it's (almost) possible. As I was doing those benchmarks, I included a Of course I must not get too carried away here with C++, LevelJS awaits! |
ok well NPM install is easy. I thought you were going to embed it wit the god design I was looking at the NodeJS of rails last iht. Its called Sails and acts as they have adapters for many differnt DB's and its a very smooth and easy i reckon a plugin for them at the CORE level woudl be amazing g On 8 March 2013 15:06, Rod Vagg [email protected] wrote:
Contact details: |
also plugins for GIS spatial stuff will be able to b put into the leveldown It will be really interesting to see how we can get master master g On 8 March 2013 15:26, Ged Wed [email protected] wrote:
Contact details: |
@rvagg I have zero objection to the underlying machinery needed to standardize the authoring and creating of plugins from a module author point of view. I think all plugin authors should create them in a similar fashion. I have an objection to the user facing API for people that use plugins instead of require('upper-rangedel').use()
db.delRange(...) I'd be far nicer to have var delRange = require('upper-rangedel')
delRange(db, ...) The reasoning for this is that it's explicit and less magical plugins that mutate global db even if AT THE IMPLEMENTATION LEVEL plugins mutate the global db |
Right, I see what you mean by 'global' then. Btw, my implementation doesn't force mutating the global, you can do it only for specific instances too: And, re the LevelDOWN version, yes I think it might be possible to do your preferred approach there too rather than injecting directly into the instance. But I guess ultimately it comes down to preferences and I'd like to hear more voices on this topic before anything happens. I maintain that my approach provides more consistency and simplicity for end-users who just want to build stuff and not be too concerned with how LevelUP and it's ecosystem works. "Just give me a database with these features, I have awesome stuff to build". |
This is interesting. When we originally discussed In the case of something like rangeDel which just iterates over a range, doing deletes, couldn't you just pass the C object to the range delete plugin?
passing leveldown to the plugin could be simpler here. However, this issue is discussing two quite different things - C++ leveldown plugins, and particular extension points. I think @Raynos' concern is valid, adding extensions willy nilly is gonna end in pain. I described the plugin approach I was using here, https://github.com/rvagg/node-levelup/wiki/Plugin-Pattern-Discussion, It was possible to create all sorts of interesting extra features with that approach, but I ended up duplicating code for managing ranges and prefixes in every plugin, they where messy, and hard to perfect. I have recently been working on a new approach, based on level-sublevel As this issue is already covering several topics, I'll start a new issue. |
my revised plugin approach: https://github.com/rvagg/node-levelup/issues/97 |
one thing I've needed to do is hook into a |
On your last question, this is from the externs_example.js, a simple, sneak plugin that cancels a var sneakyextern = {
put: function (key, options, valueEnc, callback, next) {
if (key == 'foo2') {
// bypass the real get() operation, jump straight to the user callback
return callback()
}
// internal next() callback for the extern chain
next(key, options, valueEnc, callback)
}
} You'd just extend this a little and get this: var batchplugin = {
put: function (key, options, valueEnc, callback, next) {
this.batch([ { type: 'put',
if (key == 'foo2') {
// bypass the real get() operation, jump straight to the user callback
return callback()
}
// internal next() callback for the extern chain
next(key, options, valueEnc, callback)
}
} But I thought that a more interesting example would be in order so I created an indexing plugin using the externr approach. See https://github.com/rvagg/node-levelup/blob/pluggability/externs_example.js The example stores this data (top npm packages): { {
name : 'underscore'
, author : 'jashkenas'
, version : '1.4.4'
, url : 'https://github.com/documentcloud/underscore'
}
, {
name : 'async'
, author : 'caolan'
, version : '0.2.6'
, url : 'https://github.com/caolan/async'
}
, {
name : 'request'
, author : 'mikeal'
, version : '2.14.0'
, url : 'https://github.com/mikeal/request'
}
/* duplicate indexed properties are left as an exercise for the reader!
, {
name : 'coffee-script'
, author : 'jashkenas'
, version : '1.6.1'
, url : 'https://github.com/jashkenas/coffee-script'
}
*/
, {
name : 'express'
, author : 'tjholowaychuk'
, version : '3.1.0'
, url : 'https://github.com/visionmedia/express'
}
, {
name : 'optimist'
, author : 'substack'
, version : '0.3.5'
, url : 'https://github.com/substack/node-optimist'
} and then it enables the database to be operated on like this, note the new // a standard get() on a primary entry
db.get('underscore', function (err, value) {
if (err) throw err
console.log('db.get("underscore") =', JSON.stringify(value))
})
// some gets by indexed properties
db.getBy('author', 'jashkenas', function (err, value) {
if (err) throw err
console.log('db.getBy("author", "jashkenas") =', JSON.stringify(value))
})
db.getBy('author', 'mikeal', function (err, value) {
if (err) throw err
console.log('db.getBy("author", "mikeal") =', JSON.stringify(value))
})
db.getBy('version', '0.2.6', function (err, value) {
if (err) throw err
console.log('db.getBy("version", "0.2.6") =', JSON.stringify(value))
}) producing this output:
tell me that's not awesome! |
and in that example's case, the plugin is all of 28 lines without comments; of course it's incomplete but it works nicely as an example of what's possible. |
Sorry. That example is here: https://github.com/rvagg/node-levelup/blob/pluggability/externs_example2.js The db init code is simply this: , db = levelup('/tmp/indexer.db', {
// inject the plugin
use : indexer
// our plugin reads this option
, indexProperties : [ 'author', 'version' ]
, keyEncoding : 'utf8'
, valueEncoding : 'json'
}) Note how it is able to take options of its own. |
Why can we not have simple things like
I think we should definitely have low level hooks & extensions points for leveldb MODULE authors. But those things are implementation details. But we shouldn't encourage a magical use and just drop all the properties in one big massive config hash approach. |
@rvagg Very nice! But I think @Raynos has a point. Lets say I'm completely new to LevelUP and want to use this new cool indexing module that I heard so much about. It's hard to see the connection between the values in the options hash and the actual plugin, and if there are more plugins used, then this config is going to be massive. What if some other plugin also uses the 'indexProperties' property? |
@Raynos I have a feeling we're talking about slightly different things here. This is only about module authors, being able to make modules that extend levelup in a consistent way that provide module users with a simple and consistent approach to using plugins together. There's no massive config hash, I don't know where you're seeing that. Plugins are objects that expose the extension points they care about, they don't get munged in together into some big hash. Plugin extension points extend from each other so when you add a plugin with the externr mechanism they form a chain with one executing after the other for the same extension point. So, when you have an db instance that has an "index" plugin, it can also have a totally different plugin working at the same time and the "indexer" doesn't have to care about functionality beyond its own stuff--in your approach, how do you extend on top of your "indexer" without reimplementing large chunks of levelup or saying that it can't be extended itself so you can only have indexer and nothing else significant on it as well? I'd appreciate it if you had a look at externr and try to understand what it's doing rather than dismissing this approach so easily. |
@ralphtheninja the var db = levelup('indexed.db', {
use : indexer([ 'author', 'version' ])
, keyEncoding : 'utf8'
, valueEncoding : 'json'
}) It's the var db = levelup('indexed.db', {
use : [ geospatial, indexer([ 'author', 'version' ]), rangeDel ]
}) Or, if that's not to your taste: var db = levelup('indexed.db')
db.use([ geospatial, indexer([ 'author', 'version' ]), rangeDel ]) or var db = levelup('indexed.db')
db.use(require('levelup-geospatial'))
db.use(require('levelup-indexer')([ 'author', 'version' ]))
db.use(require('upper-rangedel')) The point being, these all have the same mechanism for working inside of levelup and can play happily together and the user doesn't have to get lost in this kind of cruft: var db = levelup('indexed.db')
db = require('levelup-geospatial')(db)
var indexedDb = require('levelup-indexer')(db, [ 'author', 'version' ]) // ooops, can I extend the geospatial version of the db?
var rangeDel = require('upper-rangedel')
rangeDel(db) // which instance of `db` am I working on here? can it handle this? |
I like this version the most: var db = levelup('indexed.db')
db.use(require('levelup-geospatial'))
db.use(require('levelup-indexer')([ 'author', 'version' ]))
db.use(require('upper-rangedel')) How about not having to use require? var db = levelup('indexed.db')
db.use('levelup-geospatial')
db.use('levelup-indexer', [ 'author', 'version' ])
db.use('upper-rangedel') |
var db = levelup('indexed.db')
var getBy = require('levelup-indexer')(db, [ 'author', 'version' ])
getBy("author", "raynos", cb); /* will just work */
var rangeDel = require('upper-rangedel')
rangeDel(db, { /* some range */ }) /* this will just work */ I simply think this api is nicer then the I prefer the attitude of "we have a module system and a bunch of leveldb compatible modules" versus "hey guys we have a plugin system. you can write plugins for our platform" The former says we write commonJS modules that work with leveldb and follow the node style, if you know how to write modules, you know how to make a modular database. The latter says come use our special plugin system that's different from every other plugin system for every other framework. It's kind of like node based commonJS modules, but not! |
the standard commonJS way of doing it is also less limitting i feel. plugin systems MUST get the API right for all possible intended and On 9 March 2013 23:27, Raynos [email protected] wrote:
Contact details: |
Also if the plugin API is only used by module AUTHORS. And not by leveldb USERS then we can make more changes on it in an aggressive fashion. Because honestly most of the initial module AUTHORS are you used to this idea being experimental. The module authors can then change the internals of their modules to match the new plugin / extension API and not break back compat on their surface api. of course there are disadvantages to this. The |
yes. Private API and a Public API. Its a good way to give room to grow. g On 9 March 2013 23:45, Raynos [email protected] wrote:
Contact details: |
Posting in a new issue instead of #68 or #80 because this is about a specific implementation that I'd like comment on.
The LevelUP pluggability branch contains an extension to 0.6.x that uses externr to expose some basic extension points (more are possible of course). I've put an example in that branch here that places prefixes on keys (and removes them on the way out), and also prevents reading of certain other keys, purely to provide an example.
The externr approach is quite different from the level-hooks approach so I don't imagine this to be non-controversial. My hope is that the two approaches can coexist and perhaps leverage off each other.
While externr is pretty fast for the 'noop' case where you don't have a plugin operating on a given extension point, my guess is that it'll undo the performance gains in #90 (which haven't made it into a release yet fyi).
The way LevelUP extensions work is with a "use" call or option. There is a global
levelup.use(plugin)
that you can register plugins with any LevelUP instance created after that point. There is a"use"
property on theoptions
argument tolevelup()
when you're making a new instance, the property can point to a single plugin or an array of plugins. That instance will then use those plugins. Each instance will also expose a.use()
method that can take single or arrays of plugins, so you could add plugins after an instance is created.Plugins are simply objects whose keys are the extension points it wishes to inject itself in to. The values are functions that do the work. See the example linked to above to see what I mean.
The LevelDOWN plugins branch contains additions the implement a basic plugin system for the native layer by way of providing a Plugin class that can be extended. So far it has only one extension point, an
Init(database)
method that passes a newly created LevelDOWN database instance (i.e., when you callleveldown(location)
). Plugins can then do what they like on thedatabase
object, mostly just adding methods or replacing existing ones I suspect. But I imagine also being able to offer a mechanism to insert a particular LevelDB comparator if you need your db sorted in a particular way. Or even more advanced, a replacement LevelDB filter if you have something more efficient than the default bloom filter.Currently the way you put a plugin in LevelDOWN is with a global
leveldown._registerPlugin(location)
call, wherelocation
is the path to a .node object file it candlopen()
. Once loaded, the plugin is placed into a list of plugins and when needed the list is iterated over and each plugin has the appropriate method invoked (currently justInit()
on instance creation).Extending LevelDOWN is quite tricky, I'm not aware of any other native build offering plugins. So there are a few challenges. Currently there's an npm issue that's preventing it from being a seamless thing.
My working example is a range delete which I decided could be implemented outside of LevelUP/LevelDOWN and offered as a plugin. @Raynos has level-delete-range but when you have to do individual deletes on callbacks from an iterator it's majorly inefficient; you really want to be doing bulk deletes in one go, native & async.
Enter Downer RangeDel. I've exposed a
.use()
function on the exports so you have to explicitly run that to make it inject itself into the nearest leveldown it can find (hopefully there's just one, peerDependencies should help with that). When a LevelDOWN instance is created, it attaches a.rangeDel()
method to the object. The plugin is able to reuse a lot of the existing LevelDOWN code for iterators so the.rangeDel()
method has an almost identical options signature to a.readStream()
in LevelUP. Doing the actual delete is a simple 5-line job but it's efficient and done in one go with no callback until it's completed.Then, to make that available to LevelUP, I have Upper RangeDel. It has downer-rangedel as a dependency so you only need to load it alongside levelup to get going. I've also exposed a
.use()
method there so you have to explicitly invoke it too. It'll inject itself globally into LevelUP so it'll run in every LevelUP instance you create (but, you could opt out of global and levelup({ use: require('upper-rangedel') }) for an individual instance.The code for this should be a bit easier to understand cause it's all JS. The plugin simply extends the "constructor" of each LevelUP instance and passes the call down to
this._db.rangeDel()
where it expects that Downer RangeDel has put a method. I've also added some arguments checking and mirroed the deferred-open functionality in the rest of LevelUP so you could do something like:levelup('foo.db').rangeDel()
and it should work.To show it in action, I have an example of Upper RangeDel in work. It uses "dev" tagged levelup and leveldown releases in npm as this is not available in current "latest" releases.
package.json
index.js
Output
You can run this now, but you'll have to do a second
npm install
after the first one finishes with a failure; this is something we'll need to overcome in npm or with some crazy hackery with gyp or npm pre/postinstalls.This obviously needs more polish and thought before its production ready, but I also want to make sure I have some kind of agreement on the approach before I push ahead. So I need your thoughts!
The text was updated successfully, but these errors were encountered: