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

Race condition #68

Open
ralphtheninja opened this issue Jun 30, 2018 · 5 comments · May be fixed by #116
Open

Race condition #68

ralphtheninja opened this issue Jun 30, 2018 · 5 comments · May be fixed by #116
Labels
bug Something isn't working
Milestone

Comments

@ralphtheninja
Copy link
Member

screenshot from 2018-06-30 22-14-30

@ralphtheninja ralphtheninja added the bug Something isn't working label Jun 30, 2018
@ralphtheninja ralphtheninja added this to Backlog in Level (old board) via automation Jun 30, 2018
@ralphtheninja ralphtheninja added this to the v4.0.0 milestone Jul 1, 2018
@ralphtheninja ralphtheninja moved this from Backlog to To Do in Level (old board) Jul 1, 2018
@ralphtheninja ralphtheninja moved this from To Do to In progress in Level (old board) Jul 4, 2018
@ralphtheninja ralphtheninja self-assigned this Jul 4, 2018
@ralphtheninja ralphtheninja moved this from In progress to To Do in Level (old board) Jul 10, 2018
@vweevers
Copy link
Member

@ralphtheninja https://github.com/sinonjs/lolex might be useful for the tests here

@ralphtheninja
Copy link
Member Author

I left level-ttl for a while, but want to go back to it once we're done with abstract-leveldown.

@ralphtheninja ralphtheninja removed their assignment Jul 15, 2018
@ralphtheninja ralphtheninja moved this from To Do to Backlog in Level (old board) Jul 15, 2018
@vweevers
Copy link
Member

vweevers commented Apr 6, 2019

Good news: it looks like the recent changes in leveldown (deferring close with pending put or iterator, or something else) decreases the likelihood of the race issue. If I change the tests to use memdown the race issue happens more often.

That gives us a lead.

@vweevers
Copy link
Member

vweevers commented Apr 6, 2019

Correction, memdown failed tests because its iterator yields a different key type than leveldown. Trying to figure out why - before further investigating the race issue because there could be a larger issue in subleveldown or other.

@vweevers
Copy link
Member

vweevers commented Apr 6, 2019

Okay. The "problem" with memdown is that, when wrapped with encoding-down and the utf8 encoding and you put() a Buffer, then you'll always get back a Buffer because memdown only uses keyAsBuffer to determine whether it should convert a stored string to a requested Buffer (when keyAsBuffer is true), and not whether to convert a stored Buffer to a string (when keyAsBuffer is false, which is the case with the utf8 encoding).

See Level/community#58 (comment) for an illustration.

I could argue that this makes sense - typically you want the output type to match the input type and level-ttl is at fault for mixing string and Buffer keys - but in any case it's not a situation we can escape from atm. So, long story short, level-ttl (or at least its tests, because it looks like the problem only applies to internally used keys) should take care to convert keys to whatever type is expected.

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
Status: Backlog
Development

Successfully merging a pull request may close this issue.

2 participants