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

Clean up lifecycle thread #10480

Merged
merged 1 commit into from Aug 1, 2016
Merged

Clean up lifecycle thread #10480

merged 1 commit into from Aug 1, 2016

Conversation

dang
Copy link
Contributor

@dang dang commented Jul 28, 2016

Signed-off-by: Daniel Gryniewicz dang@redhat.com

@dang dang added the rgw label Jul 28, 2016
@yehudasa
Copy link
Member

lgtm

@cbodley
Copy link
Contributor

cbodley commented Jul 29, 2016

still a lot of failures in http://pulpito.ceph.com/cbodley-2016-07-28_15:39:31-rgw-lifecycle-cleanup---basic-mira/. hard to tell which are actually related

testing does show an issue, though - RGWLC::LCWorker::stop() just signals the thread, but doesn't actually tell it to shut down. so the logs show this:

2016-07-29 10:00:11.931628 7fa040cfa700 -1 received  signal: Terminated from  PID: 1899 task name: -bash  UID: 1000
2016-07-29 10:00:11.931648 7fa040cfa700  1 handle_sigterm
2016-07-29 10:00:11.931670 7fa040cfa700  1 handle_sigterm set alarm for 120
2016-07-29 10:00:11.931684 7fa07079aac0 -1 shutting down
...
2016-07-29 10:00:12.141148 7fa0424fd700  5 life cycle: start
...
2016-07-29 10:00:12.322323 7fa0424fd700  5 life cycle: stop
2016-07-29 10:00:12.322346 7fa0424fd700  5 shedule life cycle next start time: Sat Jul 30 00:00:00 2016

looks like this part was unfinished:

bool RGWLC::going_down()
{
  return false;
}

cbodley
cbodley reviewed Jul 29, 2016
View reviewed changes
src/rgw/rgw_rados.cc
@@ -3176,6 +3176,12 @@ void RGWRados::finalize()
delete gc;
gc = NULL;

if (use_lc_thread) {
lc->stop_processor();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

~RGWLC() is already calling stop_processor(), so we shouldn't need this part

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the same way that GC is, so maybe leave it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

k

@cbodley
Copy link
Contributor

cbodley commented Jul 29, 2016

@dang i pushed a fix for this to https://github.com/cbodley/ceph/commits/pr/10480, feel free to cherry-pick cbodley@08140f6

@dang
Clean up lifecycle thread
bc299e5 
Make sure that the lifecycle thread is terminated and all memory is
cleaned up on shutdown.

Signed-off-by: Daniel Gryniewicz <dang@redhat.com>
@dang
Copy link
Contributor Author

dang commented Jul 29, 2016

@cbodley Better fix, I think, taken from GC.

@cbodley cbodley merged commit e3ba5e8 into master Aug 1, 2016
@cbodley cbodley deleted the lifecycle-cleanup branch August 1, 2016 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@cbodley cbodley

Labels
rgw
Projects
None yet
Milestone
No milestone
3 participants
@dang @yehudasa @cbodley