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

common: ignore SIGHUP prior to fork #35844

Merged
merged 1 commit into from
Jul 15, 2020
Merged

common: ignore SIGHUP prior to fork #35844

merged 1 commit into from
Jul 15, 2020

Conversation

wuhongsong
Copy link

@wuhongsong wuhongsong commented Jun 30, 2020 •
edited
Loading

ceph-fuse process is terminated by the logratote task and what is more serious is that one Uninterruptible Sleep process will be produced

Fixes: http://tracker.ceph.com/issues/46269
Signed-off-by: hzwuhongsong wojiaowugen@163.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard backend
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@wuhongsong wuhongsong changed the title ceph-fuse: ceph-fuse process is terminated by the logratote task and … ceph-fuse: ceph-fuse process is terminated by the logratote task Jun 30, 2020
@wuhongsong wuhongsong changed the title ceph-fuse: ceph-fuse process is terminated by the logratote task ceph-fuse: ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process Jun 30, 2020
lxbsz
lxbsz reviewed Jun 30, 2020
View reviewed changes
src/ceph_fuse.cc Outdated
@@ -161,6 +161,7 @@ int main(int argc, const char **argv, const char *envp[]) {
return r;
}
if (forker.is_parent()) {
signal(SIGHUP, SIG_IGN);
Copy link
Member

Choose a reason for hiding this comment

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

Instead of fixing it here we'd better fix it in the forker.prefork(), not only the ceph-fuse have the problem.

Copy link
Author

Choose a reason for hiding this comment

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

Instead of fixing it here we'd better fix it in the forker.prefork(), not only the ceph-fuse have the problem.

Thank you for your review and reply, I have modified it.

@batrick batrick added cephfs Ceph File System needs-review labels Jun 30, 2020
@wuhongsong
Copy link
Author

jenkins test make check

batrick
batrick requested changes Jul 7, 2020
View reviewed changes
src/common/Preforker.h Outdated Show resolved Hide resolved
@wuhongsong wuhongsong requested review from batrick and lxbsz July 8, 2020 02:49
@batrick batrick added the common label Jul 8, 2020
@batrick batrick requested a review from tchaikov July 8, 2020 04:00
batrick
batrick requested changes Jul 8, 2020
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

This is fixing a bug in ceph-fuse but the commit title prefix should be common: .

@batrick
Copy link
Member

batrick commented Jul 8, 2020

Otherwise LGTM.

@wuhongsong wuhongsong changed the title ceph-fuse: ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process common: ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process Jul 8, 2020
@wuhongsong wuhongsong requested a review from batrick July 8, 2020 04:42
@wuhongsong
Copy link
Author

@tchaikov hi, kefu. Can you help to have a review？Thank you.

batrick
batrick reviewed Jul 10, 2020
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

One small nit regarding the commit title. It's be better to use an imperative style like:

common: ignore SIGHUP prior to fork

Otherwise, the ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process.

Here's my usual copy-paste information on that:

Please title your commits according to the conventions of this project. This is required for your PR to be merged.

See this article on GitHub on how to amend commits and update your pull request.

@wuhongsong wuhongsong changed the title common: ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process common: ignore SIGHUP prior to fork Jul 11, 2020
@wuhongsong
Copy link
Author

One small nit regarding the commit title. It's be better to use an imperative style like:

common: ignore SIGHUP prior to fork

Otherwise, the ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process.

Here's my usual copy-paste information on that:

Please title your commits according to the conventions of this project. This is required for your PR to be merged.

See this article on GitHub on how to amend commits and update your pull request.

Thank you for your advice, It's very important to me. I have got it.

tchaikov
tchaikov reviewed Jul 11, 2020
View reviewed changes
src/common/Preforker.h Outdated Show resolved Hide resolved
tchaikov
tchaikov reviewed Jul 13, 2020
View reviewed changes
src/common/Preforker.h Outdated Show resolved Hide resolved
@wuhongsong
common: ignore SIGHUP prior to fork
385efa7 
Otherwise, the ceph-fuse process is terminated by the logratote task and will produce one Uninterruptible Sleep process.

Fixes: http://tracker.ceph.com/issues/46269
Signed-off-by: hzwuhongsong <hzwuhongsong@corp.netease.com>
@tchaikov
Copy link
Contributor

failures tracked / fixed by

@tchaikov tchaikov merged commit fb5ec29 into ceph:master Jul 15, 2020
@smithfarm smithfarm mentioned this pull request Jul 18, 2020
Merged
2 tasks
wjwithagen added a commit to wjwithagen/ceph that referenced this pull request Jul 18, 2020
@wjwithagen
common: include <signal.h> in Preforker.h
93e2ad6 
Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier                                             'sigemptyset'
    sigemptyset(&sa.sa_mask);
    ^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for                                             initialization of 'sigaction'
    if (sigaction(SIGHUP, &sa, nullptr) != 0) {
        ^         ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via                                            ble: requires 1 argument, but 3 were provided
struct sigaction {
       ^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via                                            ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not                                             viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: ceph#35844
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
@wjwithagen wjwithagen mentioned this pull request Jul 18, 2020
Merged
3 tasks
@smithfarm smithfarm mentioned this pull request Jul 19, 2020
Merged
2 tasks
ideepika pushed a commit to ideepika/ceph that referenced this pull request Aug 7, 2020
@wjwithagen
common: include <signal.h> in Preforker.h
e62b17e 
Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier                                             'sigemptyset'
    sigemptyset(&sa.sa_mask);
    ^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for                                             initialization of 'sigaction'
    if (sigaction(SIGHUP, &sa, nullptr) != 0) {
        ^         ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via                                            ble: requires 1 argument, but 3 were provided
struct sigaction {
       ^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via                                            ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not                                             viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: ceph#35844
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
smithfarm pushed a commit to smithfarm/ceph that referenced this pull request Aug 9, 2020
@wjwithagen @smithfarm
common: include <signal.h> in Preforker.h
4e602e0 
Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier                                             'sigemptyset'
    sigemptyset(&sa.sa_mask);
    ^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for                                             initialization of 'sigaction'
    if (sigaction(SIGHUP, &sa, nullptr) != 0) {
        ^         ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via                                            ble: requires 1 argument, but 3 were provided
struct sigaction {
       ^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via                                            ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not                                             viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: ceph#35844
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
(cherry picked from commit 93e2ad6)
smithfarm pushed a commit to smithfarm/ceph that referenced this pull request Aug 9, 2020
@wjwithagen @smithfarm
common: include <signal.h> in Preforker.h
8152056 
Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier                                             'sigemptyset'
    sigemptyset(&sa.sa_mask);
    ^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for                                             initialization of 'sigaction'
    if (sigaction(SIGHUP, &sa, nullptr) != 0) {
        ^         ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via                                            ble: requires 1 argument, but 3 were provided
struct sigaction {
       ^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via                                            ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not                                             viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: ceph#35844
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
(cherry picked from commit 93e2ad6)
ideepika pushed a commit to ceph/ceph-ci that referenced this pull request Sep 3, 2020
@wjwithagen
common: include <signal.h> in Preforker.h
7de0e3b 
Since it does use functions declared by signal.h

/home/jenkins/workspace/ceph-master/src/common/Preforker.h:50:5: error: use of undeclared identifier                                             'sigemptyset'
    sigemptyset(&sa.sa_mask);
    ^
/home/jenkins/workspace/ceph-master/src/common/Preforker.h:52:9: error: no matching constructor for                                             initialization of 'sigaction'
    if (sigaction(SIGHUP, &sa, nullptr) != 0) {
        ^         ~~~~~~~~~~~~~~~~~~~~
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit copy constructor) not via                                            ble: requires 1 argument, but 3 were provided
struct sigaction {
       ^
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit move constructor) not via                                            ble: requires 1 argument, but 3 were provided
/usr/include/sys/signal.h:366:8: note: candidate constructor (the implicit default constructor) not                                             viable: requires 0 arguments, but 3 were provided
2 errors generated.

fixes: ceph/ceph#35844
Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
@wuhongsong wuhongsong deleted the fuse-assert-Dprocess branch November 16, 2023 01:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tchaikov tchaikov tchaikov approved these changes

@batrick batrick batrick approved these changes

@lxbsz lxbsz Awaiting requested review from lxbsz

Assignees
No one assigned
Labels
cephfs Ceph File System common needs-qa wip-kefu-testing
Projects
None yet
Milestone
No milestone
4 participants
@wuhongsong @batrick @tchaikov @lxbsz