feat(storage): wake up memory acquire request in order #15921
Merged
Task list completed / task-list-completed
Started
2024-05-15 08:31:53
ago
0 / 8 tasks completed
8 tasks still to be completed
Details
Required Tasks
| Task | Status |
|---|---|
| I have written necessary rustdoc comments | Incomplete |
| I have added necessary unit tests and integration tests | Incomplete |
| I have added test labels as necessary. See details. | Incomplete |
| I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features #7934). | Incomplete |
| My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future). | Incomplete |
All checks passed in ./risedev check (or alias, ./risedev c) |
Incomplete |
| My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details) | Incomplete |
| My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users) | Incomplete |
When someone else releases the quota and call may_notify_waiters, it will find A in the queue, and create a MemoryTrackerImpl with quota 100 and send to A via the tx. The send method returns successfully and the MemoryTrackerImpl is added to the buffer. |
Incomplete |
However, before A poll the rx and take the MemoryTrackerImpl out, the future of A may be cancelled and dropped, and then only the MemoryTrackerImpl gets dropped, which only adds the quota back to the atomic variable, but won't notify waiters in the queue (which is B). |
Incomplete |
| By now, B is waiting for notification, but no one is going to release any quota and notify B, which causes endless await. | Incomplete |
We make pending_request_count a atomic bool. |
Incomplete |
| We only set the atomic bool to true on first request, which we already do in this PR. | Incomplete |
We only set the atomic bool to false when the waiter queue becomes empty in may_notify_waiters. |
Incomplete |
When the quota is acquired via MemoryRequest::Ready (L287), we need to release the quota and notify waiters. |
Incomplete |
When the quota is acquired here in may_notify_waiters in L243, there are two sub-cases: |
|
a. tx.send here succeeds, regardless of whether the rx is dropped or not, we need to release quota and notify waiter. |
|
b. tx.send here fails (due to rx has been dropped before tx.send). we only need to release quota but not notify watier (Otherwise, it will deadlock). |
Incomplete |
Loading