IGNITE-20424 Added client session outbound queue size limit. #10980
Conversation
petrov-mg
force-pushed
the
IGNITE-20424
branch
3 times, most recently
from
cc06f04 to
6102e95
Compare
| } | ||
|
|
||
| /** */ | ||
| GridNioServer<ClientMessage> nioServer() { |
There was a problem hiding this comment.
I suppose that introducing a getter just for test purposes is not needed. You can get this field by reflection.
There was a problem hiding this comment.
OK. Let's use reflection. PR has been updated accordingly.
| Collection<IgniteClientFuture<byte[]>> futs = ConcurrentHashMap.newKeySet(); | ||
|
|
||
| try { | ||
| while (cliDisconnectedLatch.getCount() > 0) { |
There was a problem hiding this comment.
These curly braces are redundant, according to our codestyle
| skipClientWrite(grid(0), true); | ||
|
|
||
| Collection<IgniteClientFuture<byte[]>> futs = ConcurrentHashMap.newKeySet(); | ||
|
|
There was a problem hiding this comment.
Let us add some descriptions of what is going on in this test (i.e. flooding with get requests, etc.)
petrov-mg
force-pushed
the
IGNITE-20424
branch
from
a15e643 to
b340947
Compare
petrov-mg
force-pushed
the
IGNITE-20424
branch
from
b340947 to
0f22559
Compare
| * @return {@code this} for chaining. | ||
| */ | ||
| public ClientConnectorConfiguration setSessionOutboundMessageQueueLimit(int sesOutboundMsgQueueLimit) { | ||
| A.ensure(sesOutboundMsgQueueLimit > 0, "Session outbound queue limit must be greater than zero."); |
There was a problem hiding this comment.
Why zero is prohibited? For example we can copy configuration using copying constructor and reset limit after it. With the current restriction we can't do that.
There was a problem hiding this comment.
Removed zero restriction. Added support for sesOutboundMsgQueueLimit to the copy constructor.
|
|
||
| /** | ||
| * Test scenario: | ||
| * 1. Thin client performs huge amount of async cache get requests. It does not matter if they belong tounique keys. |
There was a problem hiding this comment.
tounique -> to unique
| /** | ||
| * Test scenario: | ||
| * 1. Thin client performs huge amount of async cache get requests. It does not matter if they belong tounique keys. | ||
| * 2. Server accepts all of them and prepares responces that are accumulated in the thin client session outbound |
There was a problem hiding this comment.
responces -> responses
| @@ -138,7 +139,8 @@ | |||
| OdbcConfigurationValidationSelfTest.class, | |||
| OdbcEscapeSequenceSelfTest.class, | |||
| SqlListenerUtilsTest.class, | |||
| JavaVersionCommandParserTest.class | |||
| JavaVersionCommandParserTest.class, | |||
| ClientSessionOutboundQueueLimitTest.class | |||
There was a problem hiding this comment.
Let's add , to the end of the string.
|
|
||
| skipClientWrite(grid(0), true); | ||
|
|
||
| Collection<IgniteInternalFuture<byte[]>> futs = ConcurrentHashMap.newKeySet(); |
There was a problem hiding this comment.
There is no concurrency here, why do we need ConcurrentHashMap?
|
|
||
| /** | ||
| * Sets Client session outbound message queue limit. Limits the number of messages waiting to be sent from the | ||
| * server side to particular thin client. If the specified limit is exceeded, corresponding thin client connection |
There was a problem hiding this comment.
In general, ClientConnectorConfiguration related not only to thin client, but also for jdbc and odbc, so perhaps it's better to use just 'client' in javadoc instead of 'thin client'
Thank you for submitting the pull request to the Apache Ignite.
In order to streamline the review of the contribution
we ask you to ensure the following steps have been taken:
The Contribution Checklist
The description explains WHAT and WHY was made instead of HOW.
The following pattern must be used:
IGNITE-XXXX Change summarywhereXXXX- number of JIRA issue.(see the Maintainers list)
the
green visaattached to the JIRA ticket (see TC.Bot: Check PR)Notes
If you need any help, please email dev@ignite.apache.org or ask anу advice on http://asf.slack.com #ignite channel.