-
Notifications
You must be signed in to change notification settings - Fork 509
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
try to fix ubuntu18 installation failure #4643
Draft
meiravgri
wants to merge
37
commits into
master
Choose a base branch
from
meiravg_fix_bionic_installation
base: master
Could not load branches
Branch not found: {{ refName }}
Could not load tags
Nothing to show
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
+138
−195
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
wait and drain are modified accordingly.
sembm om dim - 768
rename priority_queue->num_threads_working to num_threads_not_idle uncomment LOG remove script
Co-authored-by: GuyAv46 <[email protected]>
Thcount used to lock the code block in thread_do where we realize the state is THPOOL_TERMINATE_WHEN_EMPTY and the jobq, and want to inform the other threads. However, this lock was not enough to ensure that thread won't go to sleep after the signal has already been sent. for example, once the queue is empty, for thread1 both state = THPOOL_TERMINATE_WHEN_EMPTY and jobq is empty, switch to thread2: try to pull a job from the queue, is_empty & should run both true, back to thread 1: change should_run and broadcast all other threads switch to thread2: sleep on the cond we need to make sure that both code block are protected by the same mutex: 1. thread_do changes the thpool state and signal the other threads 2. jobq_pull check if the jobq is empty and should_run and sleep on the cond. we lock these code blocks with jobqueues_rwmutex. thcount_lock is now redundant and the variable it used to guard can be atomic instead.
… approximate way to signal drain that all threads are idle. However, drain is more likly to wakeup due to timeout, since idealy, all threads are working as long as there are jobs in jobq. As for wait function, the threads were signaling that all are ideal before checking the queue length, so again, it doesn't indicate the real state of the queue (num_working might be 0, but there are still jobs in the queue) the condition variable was replaced with a sleeping and waking up after 100 ms. also, num working is increased if we pulled a job, and decreased when the job is done. change back to num_threads_working added description to has_jobs and should_run remove error checking from redisearch_thpool_create add num_threads_alive to stats remove unused priority_queue_len added a test to test terminate when empty added a util to create any class of thpool tests
unify if (job_p) in thread_do remove unused var from sleep_and_set in test_cpp_thpool
remove git-man to reproduce
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4643 +/- ##
==========================================
- Coverage 85.98% 85.97% -0.01%
==========================================
Files 190 190
Lines 34517 34517
==========================================
- Hits 29678 29677 -1
- Misses 4839 4840 +1 ☔ View full report in Codecov by Sentry. |
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.
Describe the changes in the pull request
A clear and concise description of what the PR is solving, including:
Which issues this PR fixes
Main objects this PR modified
Mark if applicable