Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Iterator#dispose #7

Open
dominictarr opened this Issue · 8 comments

4 participants

@dominictarr
Owner

what sort of error can happen in iterator.dispose(cb)?

var i = db.iterator()

i.dispose(function (err) {
  //?  
})

if there is an error should I try disposing again? do I even care? I don't need the iterator any more,
so as long as it's gone now, that is all I need to know. Could iterator.dispose() take no callback?

@Raynos
Owner

anything can cause an error. it should definitely take a callback. We should enumerate all possible errors though.

For example consider this retarded example.

process.setuid(someNobody)

i.dispose(function (err) {
  if (err) {
    process.setuid(god)
    i.dispose(function (err) {
      // no error
    })
  }
})
@dominictarr
Owner

fair enough.

oh, hangon... isn't the iterator just a thing in memory? I'm not sure uid would affect that.

@Raynos
Owner

@dominictarr if the iterator actually aggresively loads the entire range into memory then wtf. Why even make it a pull stream. I expected it to do blocking IO each time you call next() with maybe some clever pre-fetching.

@juliangruber
@Raynos
Owner

@juliangruber you want to tell the iterator to cleanup once you've read a subset of the range. Otherwise it has no way of knowing whether or not your done reading.

This would basically be a rename of the current iterator#end method.

@rvagg
Owner

iterators create an implicit snapshot so you want to explicitly free it up, particularly if you're doing lots of iterating. I have a TODO in the code about returning an it->status() prior to deleting but I'm not sure how much sense that makes, I'd have to investigate but we may even have an error to pass back to a callback.

@dominictarr
Owner

@Raynos so, turns out, that snapshots are very cheap.
Since most of the database is in the form of an IMMUTABLE sst table,
the snapshot is actually only a reference to the sections that are in the memory table.

@rvagg
Owner

marking this as "help wanted", it'd be nice if someone investigated this further and gave us a resolution that can close this issue.

@rvagg rvagg added the help wanted label
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.