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

Disable Context Propagation and useless IDLE connections Netty detection #362

Merged
merged 1 commit into from
Jun 28, 2023

Conversation

franz1981
Copy link
Contributor

@cescoffier as well

Any other feedback?

@LesnyRumcajs
Copy link
Owner

@franz1981 Could you please briefly elaborate on those flags? I would like to know if they do not take advantage of the logic simplicity and are generic enough - in other words, would you use the same flags for a regular backend application? If so, is there a reason they are not the default?

Copy link

@cescoffier cescoffier left a comment

Choose a reason for hiding this comment

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

I would also test with the new gRPC server (using the http server)

@franz1981
Copy link
Contributor Author

franz1981 commented Jun 27, 2023

you use the same flags for a regular backend application? If so, is there a reason they are not the default?

3 different sets of changes here:

So, in short: assuming what the other servers are doing (they do have something to detect half-dead connections/inactive from long time?) i don't see any reason why pay for both features.

Said that, I didn't yet profiled the test nor inspected the load generator itself yet :)

@franz1981
Copy link
Contributor Author

And I've noticed another thing: https://quarkus.io/guides/grpc-service-implementation#scaling
The default number of quarkus grpc servers seems to be just 1 (vertx verticle) - meaning that what you have observed so far was probably using a single core?

@LesnyRumcajs
Copy link
Owner

And I've noticed another thing: https://quarkus.io/guides/grpc-service-implementation#scaling The default number of quarkus grpc servers seems to be just 1 (vertx verticle) - meaning that what you have observed so far was probably using a single core?

I do believe it was addressed in #361, right?

Copy link
Owner

@LesnyRumcajs LesnyRumcajs left a comment

Choose a reason for hiding this comment

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

Thanks for the write-up; I really appreciate it!

@franz1981
Copy link
Contributor Author

franz1981 commented Jun 27, 2023

I do believe it was addressed in #361, right?

Nope. Quarkus engine is using Vertx under the hood (which uses Netty), but th pr you have mentioned is on Vertx alone, not Quarkus, which have its own configuration parameters and slightly different default behaviours.

I have double checked that the runners for quarkus are instead configuring the number of instances hence my previous comment is invalid!

@LesnyRumcajs
Copy link
Owner

Sounds good, thanks!

@LesnyRumcajs LesnyRumcajs merged commit 1c99dd3 into LesnyRumcajs:master Jun 28, 2023
@He-Pin
Copy link
Contributor

He-Pin commented Jun 28, 2023

@LesnyRumcajs Would you rerun the bench on the same box some later time :) ?

@LesnyRumcajs
Copy link
Owner

@LesnyRumcajs Would you rerun the bench on the same box some later time :) ?

I'll re-run it this or the next weekend.

@franz1981
Copy link
Contributor Author

any news @LesnyRumcajs ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants