8000 exec plugin: replace select with poll by kwiatrox · Pull Request #3363 · collectd/collectd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

exec plugin: replace select with poll #3363

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
Merged
merged 1 commit into from
Feb 29, 2020
Merged

exec plugin: replace select with poll #3363

merged 1 commit into from
Feb 29, 2020

Conversation

kwiatrox
Copy link
Member
@kwiatrox kwiatrox commented Jan 7, 2020

ChangeLog: exec plugin: replace select with poll

Replace select with poll to allow big File Descriptors values on systems which allow more than 1024 open FDs. Other than that there is no any change in functionality.
This fixes issue #3322.

@kwiatrox
exec plugin: replace select with poll
ce52822 
Replace select with poll to allow big File Desciptors values
on systems which allow more than 1024 open FDs.

Change-Id: I8d5f94a19e8a299269388d45b7a84059e45cc7af
Signed-off-by: Kamil Wiatrowski <kamilx.wiatrowski@intel.com>
@kwiatrox kwiatrox force-pushed the exec_select_to_poll branch from 3384f6d to ce52822 Compare January 7, 2020 14:49
@kwiatrox kwiatrox added this to the 5.11.0 milestone Jan 7, 2020
@kwiatrox kwiatrox mentioned this pull request Jan 7, 2020
Closed
@rubenk
Copy link
Contributor
rubenk commented Jan 8, 2020

Thank you. Did you test this on all OS's we support?

@kkepka
Copy link
Contributor
kkepka commented Jan 8, 2020

I think verification of this patch may not been so comprehensive and broad in terms of multiple OS's, but will let @kwiatrox to let know about details where it has been checked.

Out of curiosity - what is actually the list of "all OS's we support"?
few of them are mentioned on https://collectd.org/features.shtml , like Linux, Solaris, Mac OS X, AIX, FreeBSD, NetBSD, and OpenBSD, but is there any more specific e.g. particular linux distributions and ver of releases?

@kwiatrox
Copy link
Member Author
kwiatrox commented Jan 9, 2020 •
edited
Loading

Thank you. Did you test this on all OS's we support?

I had a chance to test it only on Ubuntu and CentOS 7 so far.

@sunkuranganath
Copy link
Member

I would assume at this point we would go with CI checks. If CI passes and there are no pending review comments, I would think it can be merged.
As per our meetup discussions, exec plugin is part of "core plugins" list. So we require a release manager or core reviewers to accept this bug fix. Adding core reviewers (that I know of) to the review.
The sooner we accept this bug fix the better.

dago
dago approved these changes Feb 29, 2020
View reviewed changes
Copy link
Contributor
@dago dago left a comment

Choose a reason for hiding this comment

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

LGTM

@dago
Copy link
Contributor
dago commented Feb 29, 2020

Ok, I'll merge this one, but we need to keep in mind that strict release testing on all supported platforms is done.

@dago dago merged commit c4f934e into collectd:master Feb 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dago dago dago approved these changes

Assignees

@octo octo

@mrunge mrunge

Labels
core
Projects
None yet
Milestone
5.11
Development

Successfully merging this pull request may close these issues.
7 participants
@kwiatrox @rubenk @kkepka @sunkuranganath @dago @octo @mrunge
0