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

[3.11.z] Move client authentication out of generic threads #14941

Merged
merged 3 commits into from Apr 29, 2019

Conversation

Projects
None yet
3 participants
@kwart
Copy link
Contributor

commented Apr 25, 2019

This PR backports the blockingExecutor part of #13887 and uses the executor just for client authentications.

@kwart kwart force-pushed the kwart:unblockAuthn-311 branch from 7c35fa3 to c9ba045 Apr 25, 2019

@@ -104,7 +103,8 @@
public static final String SERVICE_NAME = "hz:core:clientEngine";

private static final int EXECUTOR_QUEUE_CAPACITY_PER_CORE = 100000;
private static final int THREADS_PER_CORE = 20;
private static final int BLOCKING_THREADS_PER_CORE = 20;
private static final int THREADS_PER_CORE = 1;

This comment has been minimized.

Copy link
@sancar

sancar Apr 25, 2019

Member

Is this pr final ? reducing threads per core without moving blocking tasks is dangerous.
Lots of messages will wait for threads to be available unnecessarily.

This comment has been minimized.

Copy link
@kwart

kwart Apr 26, 2019

Author Contributor

Nice catch! I missed this change during patching older version,
I'll then set 5 (blocking) + 15, does it sound reasonable, @sancar?

This comment has been minimized.

Copy link
@sancar

sancar Apr 26, 2019

Member

I think we should not change the THREADS_PER_CORE and cause regression.
Setting a new one 5 will mean that, only 5 clients can do authentication at a time. Not sure the value should be. The main purpose is not to block migration threads anyway. So 5 here should be ok.

@sancar sancar added the Team: Client label Apr 25, 2019

@kwart kwart force-pushed the kwart:unblockAuthn-311 branch from c9ba045 to f39eb17 Apr 26, 2019

@sancar

sancar approved these changes Apr 26, 2019

@mmedenjak mmedenjak added this to the 3.11.4 milestone Apr 26, 2019

@kwart kwart requested a review from mmedenjak Apr 27, 2019

@kwart

This comment has been minimized.

Copy link
Contributor Author

commented Apr 29, 2019

Thanks for reviews, guys.

@kwart kwart merged commit a78d696 into hazelcast:3.11.4 Apr 29, 2019

1 check passed

default Test PASSed.
Details

@kwart kwart deleted the kwart:unblockAuthn-311 branch Apr 29, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.