chore: Adding a mpsc intrusive queue based on Vyukov's design #562
Conversation
This file contains 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
romange
force-pushed
the
FastLoad
branch
5 times, most recently
from
82b0f82 to
7c11926
Compare
…cts. 1. Add a mpsc intrusive queue based on Vyukov's design. 2. Use it as a object pool when we pull from the queue to reuse the existing object and push into it in order to return the object back to the pool. Signed-off-by: Roman Gershman <[email protected]>
|
@dranikpg ptal. Now it's much smaller. |
dranikpg
reviewed
Dec 16, 2022
Comment on lines
1646
to
+1652
| RdbLoader::~RdbLoader() { | ||
| while (true) { | ||
| Item* item = item_queue_.Pop(); | ||
| if (item == nullptr) | ||
| break; | ||
| delete item; | ||
| } |
There was a problem hiding this comment.
I just thought that you can get a memory leak here but found an even more serious issue:
- The loader parses items and fills the shard set up to maximum with tasks
- An error occurs and stop_early_ is set, the loader returns
- The receiving fiber (for example FullSyncFb in replica) reports the error and exits, destructor runs
- You shard set has still pending tasks and they are executed, but the loader is already deleted
The solution would be removing the fast path inside Load() so that it waits for the shard set
What is more, its impossible to stop pending tasks on cancellation currently, so even after cleaning the db they'll continue inserting
But this is likely out of scope for this PR
There was a problem hiding this comment.
This scenario should be catchable with pytests on test_crash_master, probably with higher load
There was a problem hiding this comment.
You are right. Write it down somewhere.
dranikpg
approved these changes
Dec 16, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
No description provided.