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.

Sign up for GitHub

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

Jump to bottom

compaction_manager: perform_cleanup: ignore condition_variable_timed_out #15671

Closed
wants to merge 1 commit into from
Closed

compaction_manager: perform_cleanup: ignore condition_variable_timed_out #15671

wants to merge 1 commit into from

Conversation

bhalevy
Copy link
Member

@bhalevy bhalevy commented Oct 9, 2023

The polling loop was intended to ignore
condition_variable_timed_out and check for progress using a longer max_idle_duration timeout in the loop.

Fixes #15669

@bhalevy
compaction_manager: perform_cleanup: ignore condition_variable_timed_out
bfaa3a8 
The polling loop was intended to ignore
`condition_variable_timed_out` and check for progress
using a longer `max_idle_duration` timeout in the loop.

Fixes scylladb#15669

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>
@bhalevy bhalevy requested a review from nyh as a code owner October 9, 2023 07:42
@bhalevy bhalevy mentioned this pull request Oct 9, 2023
Open
@scylladb-promoter
Copy link
Contributor

CI state FAILURE - https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4131/

@scylladb-promoter
Copy link
Contributor

CI state SUCCESS - https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4141/

@avikivity
Copy link
Member

Won't that restore the problem in the original commit?

commit a5a8020
Author: Benny Halevy bhalevy@scylladb.com
Date: Mon May 8 16:49:14 2023 +0300

compaction_manager: perform_cleanup: wait until all candidates are cleaned up

cleanup_compaction should resolve only after all
sstables that require cleanup are cleaned up.

Since it is possible that some of them are in staging
and therefore cannot be cleaned up, retry once a second
until they become eligible.

Timeout if there is no progress within 5 minutes
to prevent hanging due to view building bug.

Fixes #9559

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Now we don't time out, so will we hang the view building log?

@avikivity
Copy link
Member

CI state FAILURE - https://jenkins.scylladb.com/job/scylla-master/job/scylla-ci/4131/

#15677

@bhalevy
Copy link
Member Author

bhalevy commented Oct 9, 2023 •
edited

Won't that restore the problem in the original commit?

commit a5a8020 Author: Benny Halevy bhalevy@scylladb.com Date: Mon May 8 16:49:14 2023 +0300

compaction_manager: perform_cleanup: wait until all candidates are cleaned up

cleanup_compaction should resolve only after all
sstables that require cleanup are cleaned up.

Since it is possible that some of them are in staging
and therefore cannot be cleaned up, retry once a second
until they become eligible.

Timeout if there is no progress within 5 minutes
to prevent hanging due to view building bug.

Fixes #9559

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Now we don't time out, so will we hang the view building log?

No. Like I wrote in the description, there are 2 timeouts.
A short one (10s) that caused the condition variable timeout exception that is ignored here and a longer, 5 minutes idle timeout that is checked in the loop.
The exception just caused premature exit from the loop after the short timeout expired.

@bhalevy
Copy link
Member Author

bhalevy commented Oct 12, 2023

@avikivity please ack
@nyh please review

@avikivity
Copy link
Member

ack

@bhalevy
Copy link
Member Author

bhalevy commented Oct 15, 2023

@nyh ping, please review

@bhalevy
Copy link
Member Author

bhalevy commented Oct 18, 2023

@nyh ping, please review / merge

@bhalevy
Copy link
Member Author

bhalevy commented Oct 31, 2023

@nyh reminder, please review

@bhalevy
Copy link
Member Author

bhalevy commented Nov 7, 2023

@nyh ping, please review

@bhalevy
Copy link
Member Author

bhalevy commented Nov 9, 2023

@nyh We need this fix since #15669 is blocking the testing of #14200, please complete your review

bhalevy added a commit to bhalevy/scylla that referenced this pull request Nov 13, 2023
@bhalevy
compaction_manager: perform_cleanup: ignore condition_variable_timed_out
4350f73 
The polling loop was intended to ignore
`condition_variable_timed_out` and check for progress
using a longer `max_idle_duration` timeout in the loop.

Fixes scylladb#15669

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes scylladb#15671

(cherry picked from commit 68a7bbe)
bhalevy added a commit to bhalevy/scylla that referenced this pull request Nov 13, 2023
@bhalevy
compaction_manager: perform_cleanup: ignore condition_variable_timed_out
a344c65 
The polling loop was intended to ignore
`condition_variable_timed_out` and check for progress
using a longer `max_idle_duration` timeout in the loop.

Fixes scylladb#15669

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes scylladb#15671

(cherry picked from commit 68a7bbe)
denesb pushed a commit that referenced this pull request Jan 4, 2024
@bhalevy @denesb
compaction_manager: perform_cleanup: ignore condition_variable_timed_out
d8586fd 
The polling loop was intended to ignore
`condition_variable_timed_out` and check for progress
using a longer `max_idle_duration` timeout in the loop.

Fixes #15669

Signed-off-by: Benny Halevy <bhalevy@scylladb.com>

Closes #15671

(cherry picked from commit 68a7bbe)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nyh nyh nyh approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

compaction_manager::perform_cleanup does not handle condition_variable_timed_out
4 participants
@bhalevy @scylladb-promoter @avikivity @nyh