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.

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

Detach ES using Python standard mechanism #884

Merged

Conversation

danielmitterdorfer
Copy link
Member

With this commit we use the parameter start_new_session to detach
Elasticsearch from its parent process group (Rally) instead of the
problematic (and low-level) preexec_fn. Under the hood this will use
the setsid system call instead of setpgrp.

See also:

Relates #859

With this commit we use the parameter `start_new_session` to detach
Elasticsearch from its parent process group (Rally) instead of the
problematic (and low-level) `preexec_fn`. Under the hood this will use
the `setsid` system call instead of `setpgrp`.

See also:

* https://linux.die.net/man/2/setsid
* https://linux.die.net/man/2/setpgrp

Relates elastic#859
@danielmitterdorfer danielmitterdorfer added enhancement Improves the status quo :Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch labels Jan 29, 2020
@danielmitterdorfer danielmitterdorfer added this to the 1.4.1 milestone Jan 29, 2020
@danielmitterdorfer danielmitterdorfer self-assigned this Jan 29, 2020
Copy link
Contributor

@drawlerr drawlerr left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@danielmitterdorfer
Copy link
Member Author

The CI build has failed with:

16:08:53 2020-01-29 16:08:49,724 ActorAddr-(T|:35637)/PID:13692 esrally.actor ERROR Load generator [3] has exited prematurely. Aborting benchmark.

(but only for the Python 3.5 build). Unfortunately we don't have more information. This seems unrelated to this change here though and it seems transient. I'll trigger another build.

@elasticmachine test this please

@danielmitterdorfer
Copy link
Member Author

@elasticmachine test this please

@danielmitterdorfer danielmitterdorfer merged commit 2771063 into elastic:master Jan 30, 2020
@danielmitterdorfer danielmitterdorfer deleted the start-new-session branch January 30, 2020 14:54
@danielmitterdorfer
Copy link
Member Author

Thanks for the review! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Benchmark Candidate Management Anything affecting how Rally sets up Elasticsearch enhancement Improves the status quo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants