Skip to content
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

Applies prefix twice on nested sublevel #78

Closed
vweevers opened this issue Sep 30, 2019 · 12 comments · Fixed by #81
Closed

Applies prefix twice on nested sublevel #78

vweevers opened this issue Sep 30, 2019 · 12 comments · Fixed by #81
Labels
bug Something isn't working

Comments

@vweevers
Copy link
Member

vweevers commented Sep 30, 2019

Because the prefix is applied on open:

if (subdb && subdb.prefix) {
self.prefix = subdb.prefix + self.prefix
self.leveldown = reachdown(subdb.db, matchdown, false)
} else {

So if you open, close and reopen, self.prefix = subdb.prefix + self.prefix happens twice.

There are more issues with the open logic (#60, #77), so let's refactor it a bit.

@vweevers vweevers added the bug Something isn't working label Sep 30, 2019
@vweevers vweevers added this to Backlog in Level (old board) via automation Sep 30, 2019
@vweevers vweevers moved this from Backlog to To Do in Level (old board) Oct 1, 2019
@vweevers vweevers moved this from To Do to In progress in Level (old board) Oct 5, 2019
@MeirionHughes
Copy link
Member

MeirionHughes commented Oct 7, 2019

yeah I've got this too. 4.1.0 was fine, but on 4.1.1 I get prefix duplication.

I'm trying to replicate it, but its proving troublesome as if I straight up make the subs it works. In practice my code is creating subs "on the fly" then caching a helper class that manages the subdb.

@MeirionHughes
Copy link
Member

MeirionHughes commented Oct 7, 2019

I've managed to duplicate it:

  let db = levelup(encoding(memdown()));

  let fooDb = sub(db, "foo");
  let barDb = sub(fooDb, "bar");
  let rayDb = sub(barDb, "ray"); 

  await fooDb.put("def", "one")
  await barDb.put("def", "two")

  await rayDb.put("123", "hello world");
  await rayDb.put("124", "hello world");
  await rayDb.put("125", "hello world");

result with with subleveldown@4.1.1:

 !foo!!bar!def
 !foo!!foo!!bar!!ray!123
 !foo!!foo!!bar!!ray!124
 !foo!!foo!!bar!!ray!125
 !foo!def

while with subleveldown@4.1.0:

 !foo!!bar!!ray!123
 !foo!!bar!!ray!124
 !foo!!bar!!ray!125
 !foo!!bar!def
 !foo!def

@vweevers
Copy link
Member Author

vweevers commented Oct 7, 2019

@MeirionHughes We've already got a test for it. And 4.0.0 and 4.1.0 were broken (in other ways), so while it may seem 4.1.1 has the issue, it actually revealed it, and it has been here since 3.0.0.

@vweevers
Copy link
Member Author

vweevers commented Oct 7, 2019

Although... wait. The issue you're describing is a separate one (not about re- opening) that we might be able to fix on a shorter term. It's probably a race issue between the different sublevels all wanting to open the db.

Simple workaround is to do:

let db = levelup(encoding(memdown()))

db.on('open', function () {
  // create your sublevels now
})

@MeirionHughes
Copy link
Member

MeirionHughes commented Oct 7, 2019

I've already tried with:

  let db = levelup(encoding(memdown()));

  await db.open();

  let fooDb = sub(db, "foo");
  await fooDb.open();
  
  let barDb = sub(fooDb, "bar");
  await barDb.open();

  let rayDb = sub(barDb, "ray"); 
  await rayDb.open();

and I get the same.

I was going to see what difference this really has on this 3-layer nesting.

@vweevers
Copy link
Member Author

vweevers commented Oct 7, 2019

I was going to see what difference this really has on this 3-layer nesting.

That's been replaced in a later version

@vweevers
Copy link
Member Author

vweevers commented Oct 7, 2019

OK, I'm able to reproduce your issue on master as well.

@vweevers
Copy link
Member Author

vweevers commented Oct 7, 2019

Possible fix is to replace this (which can land on subdown rather than leveldown, so we end up wrapping subdown with subdown, hence the double prefix):

self.leveldown = reachdown(subdb.db, matchdown, false)

With:

self.leveldown = subdb.leveldown

@MeirionHughes Could you try that out?

@MeirionHughes
Copy link
Member

yeah that seems to do the trick

@vweevers
Copy link
Member Author

vweevers commented Oct 7, 2019

We could release that fix as semver-patch, but consumers can in theory lose access to already stored data (with a double prefix)... does that make it a breaking change? Yolo.

@vweevers vweevers moved this from In progress to Review in Level (old board) Oct 8, 2019
Level (old board) automation moved this from Review to Done Oct 8, 2019
@vweevers
Copy link
Member Author

vweevers commented Oct 8, 2019

4.1.4

@MeirionHughes
Copy link
Member

cheers: looks like its working well and I have about five sub-levels too. Thanks for the quick fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
2 participants