my8bird / node-leveldb

NodeJS bindings to levelDB - a fast and lightweight key/value database library
http://code.google.com/p/leveldb/
BSD 2-Clause "Simplified" License
63 stars 12 forks source link

iterator should be a stream #28

Open max-mapper opened 12 years ago

max-mapper commented 12 years ago

right now with iterator.forRange there is no way to know when the iterator has finished. ideally creating an iterator return a readable Stream

max-mapper commented 12 years ago

in the meantime i am using this workaround

function getLast(cb) {
  db.iterator(function(err, iterator) {
    if (err) return cb(err)
    iterator.last(function(err) {
      if (err) return cb(err)
      iterator.current(function(err, key, val) {
        cb(err, key)
      })
    })
  })
}

then i can use the returned value as the end parameter in forRange

max-mapper commented 12 years ago

I have further improved upon the above design here: https://github.com/maxogden/plumbdb/commit/0cf5212673242413ac4a4b27858a70cd23e915c8

essentially I am using the seek, current and next functions on Iterator wrapped in a readable Stream

my8bird commented 12 years ago

I am not opposed to this but do not have much time right now. If anyone wants to take a stab at it that would great.

my8bird commented 12 years ago

@maxogden Does the patch supplied resolve your issue? It seems like a different approach to a common problem.

max-mapper commented 12 years ago

the above patch makes this library more usable but this issue should stay open as a stream would be a more ideal API

gflarity commented 12 years ago

So you mean write wrapper that iterates on your behalf and emits data events (ie a stream?). Iterating over key-values in leveldb is FAST. Imagine iterating over every key in a very large database, without back pressure I bet something would break. Run out of ram because there's too many events in the queue? Or perhaps the overhead from crossing the V8->C++ boundary would just grind your app to a screeching halt.

I can understand the desire for a stream interface, it sounds like it make more sense as helper module for people who understand that you shouldn't iterate over a large set with it. This module should match the leveldb C++ interface as much as possible IMHO.

max-mapper commented 12 years ago

streams support backpressure. I get that it should match the c++ interface but it's a node binding and streams are part of node core

carter-thaxton commented 11 years ago

@maxogden, I like your suggestion of exposing a stream. I may take a stab at this. Stay tuned.

mikepb commented 11 years ago

:+1:

max-mapper commented 11 years ago

Fwiw I've been happy with the stream implementation in rvagg/levelup

Sent from my iPhone

On Mar 7, 2013, at 12:18 PM, Michael Phan-Ba notifications@github.com wrote:

— Reply to this email directly or view it on GitHub.

carter-thaxton commented 11 years ago

Wow, rvagg/levelup looks like it's really matured, and has a nice little ecosystem. Last I looked, there wasn't an "obvious" winner among the leveldb drivers. @maxogden, in your opinion, is there any reason to resurrect this node-leveldb project, or have you been satisfied with levelup? No need to reinvent the wheel.