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

gRPC auto threadiness #1271

Merged
merged 5 commits into from
Jul 7, 2020
Merged

gRPC auto threadiness #1271

merged 5 commits into from
Jul 7, 2020

Conversation

leodido
Copy link
Member

@leodido leodido commented Jun 22, 2020 •
edited
Loading

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

NONE

What this PR does / why we need it:

This PR updates the threadiness configuration default value of the gRPC configuration block.

Now, the threadiness field is 0 by default, which means "auto".
Thus, by default, Falco will set the threadiness of the gRPC server to the number of online cores.

This is the default behavior because it has proven to behave better in the recent tests both me and @fntlnz have done.

Which issue(s) this PR fixes:

NONE

Special notes for your reviewer:

When Falco is not able to guess the number of online cores (this can happen for various reasons), the threadiness will be set to 1 which we consider the safer value and also avoids oversubscription.

Does this PR introduce a user-facing change?:

new: auto threadiness for gRPC server
update: default threadiness to 0 ("auto" behavior)

leodido and others added 4 commits June 22, 2020 15:40
@leodido @fntlnz
new: hardware_concurrency helper
d53f002 
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido @fntlnz
update(userspace/falco): gRPC server threadiness 0 by default (which
147ee7e 
means "auto")

The 0 ("auto") value sets the threadiness to the number of online cores
automatically.

Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido @fntlnz
chore(userspace): log the gRPC threadiness
bd58139 
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido @fntlnz
update: default grpc server threadiness is 0 now ("auto")
1add6b4 
Co-authored-by: Lorenzo Fontana <lo@linux.com>
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido
Copy link
Member Author

leodido commented Jun 22, 2020

/milestone 0.24.0

@poiana poiana added this to the 0.24.0 milestone Jun 22, 2020
@poiana poiana requested review from fntlnz and mfdii June 22, 2020 15:53
@poiana poiana added the size/S label Jun 22, 2020
krisnova
krisnova reviewed Jun 23, 2020
View reviewed changes
falco.yaml Show resolved Hide resolved
fntlnz
fntlnz previously approved these changes Jun 24, 2020
View reviewed changes
@poiana poiana added the lgtm label Jun 24, 2020
@poiana
Copy link
Contributor

poiana commented Jun 24, 2020

LGTM label has been added.

Git tree hash: a9f510b3480e2c87d6bfb5d565628e3725df7872

leodido
leodido commented Jun 24, 2020
View reviewed changes
falco.yaml Show resolved Hide resolved
@leodido
docs: auto threadiness comment into Falco config
fb056dd 
Signed-off-by: Leonardo Di Donato <leodidonato@gmail.com>
@leodido leodido requested a review from fntlnz June 24, 2020 09:43
@poiana poiana added the lgtm label Jun 24, 2020
@poiana
Copy link
Contributor

poiana commented Jun 24, 2020

LGTM label has been added.

Git tree hash: c28cd15def2ea52a7c742470f29d011dad296def

@fntlnz
Copy link
Contributor

fntlnz commented Jun 26, 2020

/hold

I'm holding this until we have two approvers so that the others PRs in line can get a chance to be merged.

@fntlnz
Copy link
Contributor

fntlnz commented Jun 26, 2020

/honk

@poiana
Copy link
Contributor

poiana commented Jun 26, 2020

@fntlnz:
goose image

In response to this:

/honk

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@leodido
Copy link
Member Author

leodido commented Jun 26, 2020

/meow

@poiana
Copy link
Contributor

poiana commented Jun 26, 2020

@leodido: cat image

In response to this:

/meow

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@leodido
Copy link
Member Author

leodido commented Jun 29, 2020

/hold cancel

Ready to be merged in as soon @kris-nova or any other maintainer approves 😺

@fntlnz
Copy link
Contributor

fntlnz commented Jun 30, 2020

/hold to let the other PRs go in.

leogr
leogr approved these changes Jul 7, 2020
View reviewed changes
Copy link
Member

@leogr leogr left a comment

Choose a reason for hiding this comment

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

LGTM

Great job :)

@poiana
Copy link
Contributor

poiana commented Jul 7, 2020

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fntlnz, leogr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@leodido
Copy link
Member Author

leodido commented Jul 7, 2020

/hold cancel

@poiana poiana merged commit 3bad1d2 into master Jul 7, 2020
@poiana poiana deleted the feat/grpc-auto-threadiness branch July 7, 2020 11:42
leogr added a commit to falcosecurity/evolution that referenced this pull request Jul 14, 2020
@leogr
update(deploy/kubernetes): enable gRPC auto threadiness by default
46ea814 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
@leogr leogr mentioned this pull request Jul 14, 2020
Merged
2 tasks
leogr added a commit to falcosecurity/charts that referenced this pull request Jul 17, 2020
@leogr
update(falco): enable gRPC auto threadiness
89d68a0 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
poiana pushed a commit to falcosecurity/charts that referenced this pull request Jul 17, 2020
@leogr @poiana
update(falco): enable gRPC auto threadiness
bb43de4 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
poiana pushed a commit to falcosecurity/evolution that referenced this pull request Jul 17, 2020
@leogr @poiana
update(deploy/kubernetes): enable gRPC auto threadiness by default
3a62fa7 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
leogr added a commit to falcosecurity/deploy-kubernetes that referenced this pull request Jun 7, 2022
@leogr @poiana
update(deploy/kubernetes): enable gRPC auto threadiness by default
be87b04 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
leogr added a commit to falcosecurity/contrib that referenced this pull request Aug 24, 2022
@leogr @poiana
update(deploy/kubernetes): enable gRPC auto threadiness by default
c546a00 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
leogr added a commit to falcosecurity/contrib that referenced this pull request Aug 24, 2022
@leogr @poiana
update(deploy/kubernetes): enable gRPC auto threadiness by default
9bc221a 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
maxgio92 pushed a commit to falcosecurity/evolution that referenced this pull request Sep 8, 2022
@leogr @maxgio92
update(deploy/kubernetes): enable gRPC auto threadiness by default
0570c9c 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
maxgio92 pushed a commit to falcosecurity/evolution that referenced this pull request Sep 22, 2022
@leogr @maxgio92
update(deploy/kubernetes): enable gRPC auto threadiness by default
9e76096 
As per falcosecurity/falco#1271

Signed-off-by: Leonardo Grasso <me@leonardograsso.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@krisnova krisnova krisnova left review comments

@leogr leogr leogr approved these changes

@fntlnz fntlnz fntlnz approved these changes

@mfdii mfdii Awaiting requested review from mfdii

Assignees

@fntlnz fntlnz

@leogr leogr

Labels
approved dco-signoff: yes kind/feature lgtm release-note size/S
Projects
None yet
Milestone
0.24.0
Development

Successfully merging this pull request may close these issues.
5 participants
@leodido @poiana @fntlnz @leogr @krisnova