RFC: Enhanced pessimistic lock queueing#100
Open
MyonKeminta wants to merge 8 commits into
Open
Conversation
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
30 tasks
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
MyonKeminta
marked this pull request as ready for review
September 2, 2022 02:12
ekexium
reviewed
Sep 21, 2022
| - Put them in the lock table in `ConcurrencyManager`, which means, we can use key to index both memory lock and the lock waiting queue (but of course they should not share the same mutex). In this way, we tries to put concurrency-managing stuff together, which is logically reasonable. But it makes cleaning up of entries in the lock table more complicated. Besides, it have risk of reducing performance of scanning memory locks. | ||
| - Put them in a separated concurrent hashmap indexed by key. It should be simpler and more efficient than putting into lock table. | ||
|
|
||
| As we will explain later, when a request is waiting in queue, it's possible that it exceeds its timeout. `WaiterManager` will be responsible to cancel it. However, |
Signed-off-by: MyonKeminta <MyonKeminta@users.noreply.github.com>
ekexium
reviewed
Sep 22, 2022
|
|
||
| #### Lazy-cleaning up | ||
|
|
||
| As we will explain later, when a request is waiting in queue, it's possible that it exceeds its timeout, or encounters deadlock. `WaiterManager` will be responsible to cancel it. However, the corresponding entry in the queue won't be removed from the priority queue efficiently. Therefore, we can use a lazy-cleaning up approach: when popping an entry from a queue, if the entry is cancelled but it is canceled (which can be indicated by an atomic flag), drop it and continue popping the next. |
Contributor
There was a problem hiding this comment.
when popping an entry from a queue, if the entry is cancelled but it is canceled typo?
ekexium
reviewed
Sep 22, 2022
|
|
||
| We put the queues in a concurrent hashmap indexed by key, and put the hashmap inner the `Scheduler`. | ||
|
|
||
| #### Lazy-cleaning up |
Contributor
There was a problem hiding this comment.
Removal of an element is O(log n), whether lazily or not. Considering the problem mentioned, would it be better to just remove elements immediately when cancelling?
Contributor
Author
There was a problem hiding this comment.
When a request is canceled, it might not be at the head of the queue. It's possible to remove any element from a priority queue in O(log n) time, but it doesn't seem to be supported by the std BinaryHeap. I'm actually considering introducing a third-party implementation of priority queue (didn't find any proper one yet) or implement it by myself.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref: tikv/tikv#13298