mina-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guus der Kinderen (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DIRMINA-1078) OrderedThreadPoolExecutor should allow sessions to be prioritized
Date Mon, 19 Feb 2018 09:09:04 GMT

    [ https://issues.apache.org/jira/browse/DIRMINA-1078?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16368890#comment-16368890
] 

Guus der Kinderen commented on DIRMINA-1078:
--------------------------------------------

Thanks for the feedback guys! I initially had this as a separate {{Executor}}, exactly for
the reasons that you mention. You can see this variant of the code here: https://github.com/guusdk/mina/commit/c0a421cf445696fbfd4d5b10d650d7c71d8faab7
(note that I fixed a bug later on - don't blindly use this version!)

There was a _lot_ of code duplication going on. I thought about creating an abstract base
class from which this implementation, but also  {{OrderedThreadPoolExecutor}} would extend
- but that proved to much of a hassle.

With the patch that I introduced, I believe I was able to add the new functionality to the
existing implementation of {{OrderedThreadPoolExecutor}}, as a completely _optional_, _not
enabled by default_ behavior, that introduces next to _no behavioral changes_ if unused. Given
this, I would personally prefer my addition to remain in {{OrderedThreadPoolExecutor}} instead
of introducing a different {{ThreadPool}} implementation, but that's not a hill I'm willing
to die on.

[~johnnyv], the {{FIFOEntry}} implementation is used as a tie-breaker, in case the {{Comparator}}
that is provided does not distinguish between the sessions. It arguably can be left out, but
I'd prefer to keep it in: It allows one to use relatively simple {{Comparator}} implementations,
while still be ensured of defined behavior for those elements that are not covered in the
{{Comparator}}. Again, not a hill I'm willing to die on though.

[~elecharny], if you're willing to transform my patch into a separate {{Executor}} (in case
my arguments above were not compelling), please, be my guest. If anything, it'd save me time,
and having a second set of eyes looking at this code will not hurt either.

> OrderedThreadPoolExecutor should allow sessions to be prioritized
> -----------------------------------------------------------------
>
>                 Key: DIRMINA-1078
>                 URL: https://issues.apache.org/jira/browse/DIRMINA-1078
>             Project: MINA
>          Issue Type: New Feature
>          Components: Core
>            Reporter: Guus der Kinderen
>            Priority: Minor
>         Attachments: 20180216.patch
>
>
> The functionality provided in {{OrderedThreadPoolExecutor}} should be augmented to, optionally,
allow for assignment of priority to certain sessions over others.
> We've introduced this functionality after observing issues in a deployment where system
resources where being starved by the sheer amount of sessions that attempted to perform TLS.
Without the class introduced by this commit, events for any session could eventually fail
(time-out), causing the session to fail. If that session happened to be a session that had
already established TLS, the resources that were spent on establishing TLS are wasted. The
negative effect is amplified by the fact that a typical client in such a situation would attempt
>  to reconnect, which further adds to the load of the system already being starved.
> With the modifications introduced by the patch provided in this issue, priority can be
given to sessions that have already established TLS. This dramatically reduces the issue described
above, as the first sessions to fail will be those that are still negotiating TLS. Using a
specialized {{Comparator}}, one can even prioritize between these, causing sessions for which
least effort has performed to fail before sessions that are more likely to near TLS completion.
> The patch intends to add this feature as optional functionality to the existing implementation,
with little side effects to the existing, default behavior.
> The implementation provided here was initially based on a copy of the implementation
of {{OrderedThreadPoolExecutor}} that introduced a considerable amount of code duplication.
For illustrative purposes, the line of commits leading from that initial commit to the patch
attached to this JIRA issue can be found at [https://github.com/guusdk/mina/commit/c0a421cf445696fbfd4d5b10d650d7c71d8faab7]
and later commits.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Mime
View raw message