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

Insert-during-iterate deadlock #74

Open
marvin-j97 opened this issue Aug 3, 2024 Discussed in #73 · 2 comments
Open

Insert-during-iterate deadlock #74

marvin-j97 opened this issue Aug 3, 2024 Discussed in #73 · 2 comments
Assignees
Labels
api bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed

Comments

@marvin-j97
Copy link
Collaborator

Discussed in #73

Originally posted by dbbnrl August 3, 2024
In my application I sometimes generate DB entries from existing entries. The pattern is that I iterate over a range of existing keys, occasionally creating new entries (outside the iterator range) as I go.

It looks like Fjall is deadlocking in this scenario after some number of insertions. I thought perhaps using the snapshot feature would help, but it makes no difference.

Glancing at the code, I can see that create_iter takes a bunch of Read locks which it holds until the iterator is dropped. Presumably the insert code is attempting to grab a Write lock at some point (memtable flush?) so this isn't a surprising outcome...

I thought about reporting this as an issue but if this kind of usage is an explicit non-goal of Fjall I didn't want to do that.

Is this supported? Supportable?

@marvin-j97 marvin-j97 added the bug Something isn't working label Aug 3, 2024
@marvin-j97 marvin-j97 self-assigned this Aug 3, 2024
@marvin-j97 marvin-j97 added the help wanted Extra attention is needed label Aug 5, 2024
@marvin-j97
Copy link
Collaborator Author

marvin-j97 commented Oct 13, 2024

Code should never ever deadlock, so if try_lock fails for the journal rotation and the memtable size is much larger than the configured limit, return an Error::WhatIsGoingOn (not literally), maybe

@marvin-j97 marvin-j97 added api good first issue Good for newcomers labels Oct 13, 2024
@marvin-j97
Copy link
Collaborator Author

marvin-j97 commented Oct 19, 2024

  1. Add try_rotate_active_memtable to lsm-tree

  2. On lock error, check memtable size

  3. If memtable size > 110% * max_memtable_size, we probably have an insert-while-iterate scenario, so return Error::Overpressure [1]

  4. add a unit test with a function that loops forever, inserting stuff while having an iterator open... that function should return Error::Overpressure and not panic or deadlock (as it would now)

[1] Because try_lock may be prevented by open reads, we may need a fair RwLock, so the writers don't starve...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api bug Something isn't working good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

1 participant