Skip to content
This repository has been archived by the owner on Dec 2, 2024. It is now read-only.

snapshots seem not to work #43

Closed
juliangruber opened this issue Jul 7, 2015 · 4 comments · Fixed by #68
Closed

snapshots seem not to work #43

juliangruber opened this issue Jul 7, 2015 · 4 comments · Fixed by #68
Labels
bug Something isn't working

Comments

@juliangruber
Copy link
Member

this test (simplified for this issue):

test('iterator create snapshot correctly', function (t) {
    db.put('foobatch1', 'bar1', function(err){
      t.error(err)
      var iterator = db.iterator()
      db.del('foobatch1', function () {
        iterator.next(function (err, key, value) {
          t.notOk(err, 'no error')
          t.ok(key, 'got a key')
          t.equal(key.toString(), 'foobatch1', 'correct key')
          t.equal(value.toString(), 'bar1', 'correct value')
          iterator.end(t.end.bind(t))
        })
      })
    })
  })

is failing:

# iterator create snapshot correctly
default_stream.js:27 ok 419 no error
default_stream.js:27 not ok 420 got a key
default_stream.js:27   ---
default_stream.js:27     operator: ok
default_stream.js:27     expected: true
default_stream.js:27     actual:   undefined
default_stream.js:27     at: Test.assert (http://localhost:9966/test.js:12977:17)
default_stream.js:27   ...
iterator-test.js:467 Uncaught TypeError: Cannot read property 'toString' of undefined

@maxogden do you know what's going on here?

@juliangruber juliangruber added the bug Something isn't working label Jul 7, 2015
timkuijsten pushed a commit to timkuijsten/level.js that referenced this issue Feb 4, 2016
Fixes Level#43 Level#40 and Level#3. Unfortunately not easy to split out in separate commits.

1. Level#43 "snapshots seem not to work"

The reason snapshots didn't work in the test, is because the iterator was only
opened on the first call to _next. The new version opens the iterator right
away in the constructor (if there is no KeyRange error).

2. Level#40 "abstract-leveldown tests aren't all passing anymore"

Partially fixed by merging PR Level#42 and further fixed by this commit.

3. Level#3 "stop batch reads after limit is reached"

The solution is stop calling cursor.continue() once the limit is reached. The
transaction will automatically timeout as prescribed by the spec. idb-wrapper
can't be used for this because limit is only respected if autoContinue is true.

All 563 tests pass on:
* Safari 9.0.3
* Firefox 45.0b2
* Firefox 46.0a2
* Iridium 44.1 (Chrome/44.0.2403.157)
mvayngrib pushed a commit to mvayngrib/level.js that referenced this issue Aug 21, 2016
Fixes Level#43 Level#40 and Level#3. Unfortunately not easy to split out in separate commits.

1. Level#43 "snapshots seem not to work"

The reason snapshots didn't work in the test, is because the iterator was only
opened on the first call to _next. The new version opens the iterator right
away in the constructor (if there is no KeyRange error).

2. Level#40 "abstract-leveldown tests aren't all passing anymore"

Partially fixed by merging PR Level#42 and further fixed by this commit.

3. Level#3 "stop batch reads after limit is reached"

The solution is stop calling cursor.continue() once the limit is reached. The
transaction will automatically timeout as prescribed by the spec. idb-wrapper
can't be used for this because limit is only respected if autoContinue is true.

All 563 tests pass on:
* Safari 9.0.3
* Firefox 45.0b2
* Firefox 46.0a2
* Iridium 44.1 (Chrome/44.0.2403.157)
@ralphtheninja
Copy link
Member

ralphtheninja commented Sep 12, 2017

@juliangruber Just noticed this myself while trying to bring level.js up to par on abstract-levelown. It seems that it doesn't have support for snapshots and that needs to be implemented.

I checked memdown and it seems it grabs the tree from the db, which I think is why it works well for memdown, see https://github.com/Level/memdown/blob/19ed79fbf853571098b6475d8c79c10ca6f22d4b/memdown.js#L36

@juliangruber
Copy link
Member Author

Can we implement snapshots after we bring the rest up to par? It seems like this is it's own good chunk of work.

@vweevers
Copy link
Member

The thing with IndexedDB is that you can't have both snapshots and proper backpressure. Details in the thread starting here: #46 (comment)

@vweevers
Copy link
Member

I checked memdown and it seems it grabs the tree from the db, which I think is why it works well for memdown

Memdown is a different story. It works because it's backed by functional-red-black-tree which is a persistent data structure. I.e. iteration happens on an immutable tree, unaffected by simultaneous writes.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants