You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "peter royal (JIRA)" <ji...@apache.org> on 2006/03/05 01:45:39 UTC
[jira] Created: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Allow ThreadPool used by ThreadPoolFilter to be pluggable
---------------------------------------------------------
Key: DIRMINA-184
URL: http://issues.apache.org/jira/browse/DIRMINA-184
Project: Directory MINA
Type: Improvement
Reporter: peter royal
Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12414621 ]
Trustin Lee commented on DIRMINA-184:
-------------------------------------
The problem with extra properties is caused by the impedence mismatch between thread pools and executors. Because the executor abstracts all kinds of thread model, we can safely say it's a superset of thread pools.
What do you think about following the same way the JDK did, like creating an ExecutorFilter to get rid of this impedence mismatch. By doing this, we can retain the extra properties in ThreadPool and make it extend an Executor interface. Of course, the Executor interface should be a backported version of Java 5.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12418920 ]
Trustin Lee commented on DIRMINA-184:
-------------------------------------
No objection! Did you already make changes?
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12402308 ]
peter royal commented on DIRMINA-184:
-------------------------------------
1) Whoops! Will remove
2) Yes, the default should be changed. I left both in temporarily as to allow people to test the new one and the old side-by-side
3) Since the ThreadPool isn't specific about processing any specific type of Runnable, I thought it was appropriate to have the ThreadPool interface just take a Runnable.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12414584 ]
peter royal commented on DIRMINA-184:
-------------------------------------
Overall looks good. My only concern is the new properties on the ThreadPool, since they really only apply to the LeaderFollower implementation.. I think we should only put things on the interface that are generally applicable across implementations.
Anyone else have thoughts on this?
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=all ]
peter royal updated DIRMINA-184:
--------------------------------
Attachment: pluggable thread pool.diff
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Attachments: pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Assigned: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=all ]
peter royal reassigned DIRMINA-184:
-----------------------------------
Assign To: peter royal
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12379002 ]
Trustin Lee commented on DIRMINA-184:
-------------------------------------
Sounds good!
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12414359 ]
Trustin Lee commented on DIRMINA-184:
-------------------------------------
Peter,
Please review my changes:
http://svn.apache.org/viewvc?view=rev&revision=411052
* Moved all thread pool classes to org.apache.mina.filter.thread
* Replaced the old ThreadPoolFilter with the new one
* Added default properties to ThreadPool
Adding default properties can be controversial because we don't have any control over Executor implementations. Please feel free to comment, and let's close this issue when you're confident it's time to close this issue.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Resolved: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=all ]
peter royal resolved DIRMINA-184:
---------------------------------
Fix Version: 0.9.5
Resolution: Fixed
Committed as an alternate filter, the ThreadPoolThreadPoolFilter. (yea, bad name..) We can replace the current TPF after some testing, but I did not want to force the newer filter on anyone just yet.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Srikanth Veeramachaneni (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12370224 ]
Srikanth Veeramachaneni commented on DIRMINA-184:
-------------------------------------------------
Attached is a slight variant of the pluggable ThreadPoolFilter. It differs only slightly from the one that was already submitted in the following manner
- The SessionBuffer is maintained as an attribute of the IoSession instead of in a single map. This shifts the synchronization on the IoSession instead of on the map. Not sure if I am losing any performance by doing so. But this way, the life cycle of a SessionBuffer is linked to the life cycle of the IoSession. Only one session buffer is created for each IoSession instead of the session buffer being released every time the event queue become empty.
- Removed the need for 'queuedSessionBuffers' by adding a 'processingCompleted' flag. I assumed that the sole purpose of this queue was to know when a new runnable task has to be created for a session buffer. A subtle change is that I replaced the use of ConcurrentLinkedQueue with a LinkedList for the event queue, as I synchronize on the event queue both the times when the queue is accessed.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Attachments: pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12377100 ]
peter royal commented on DIRMINA-184:
-------------------------------------
Srikanth, great addition! I've been running tests with this for the past few days and its very solid.. great removal of another synchronization point.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Updated: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Srikanth Veeramachaneni (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=all ]
Srikanth Veeramachaneni updated DIRMINA-184:
--------------------------------------------
Attachment: NewThreadPoolFilter.java
The attachment for my previous comment.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12378939 ]
peter royal commented on DIRMINA-184:
-------------------------------------
Will add to SVN shortly, will make a new 'java5' module to contain code that can use java5 until the core depends on it.
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12417838 ]
peter royal commented on DIRMINA-184:
-------------------------------------
k, so the ThreadPoolFilter becomes an ExecutorFilter, since that's all it really needs to do its work?
the reason i avoided doing this before, is because it would change the name of something we talk about all the time :)
I don't think we need to keep the ThreadPool interface if we do that, since we only have one implementation.
I'll do this tomorrow if no objections (hackathon!)
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "peter royal (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12418954 ]
peter royal commented on DIRMINA-184:
-------------------------------------
Nope, got sidetracked with the filter to prevent read overloads, will do this next :)
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira
[jira] Commented: (DIRMINA-184) Allow ThreadPool used by
ThreadPoolFilter to be pluggable
Posted by "Trustin Lee (JIRA)" <ji...@apache.org>.
[ http://issues.apache.org/jira/browse/DIRMINA-184?page=comments#action_12402269 ]
Trustin Lee commented on DIRMINA-184:
-------------------------------------
Peter, I have a few questions about your commit:
1)
At line 187 of LeaderFollowerThreadPool:
//TODO this should be in the filter, inits on pre-add if we have not been init'ed
public void onPreAdd( IoFilterChain parent, String name, IoFilter.NextFilter nextFilter )
throws Exception
{
if( leader == null )
{
init();
}
}
Do we need this method? I guess this method is not invoked by anyone.
2) If LeaderFollowerThreadPool and ThreadPoolThreadPoolFilter provides exactly the same functionality with ThreadPoolFilter, why don't we replace it? ThreadPoolThreadPoolFilter could become a ThreadPoolFilter which uses LeaderFollowerThreadPool by default, and the pool implementation could be changed if a user specifies his or her favorite pool implementation.
3) Can't we simply expose ProcessEventRunnable to users and make ThreadPool.submit accept it instead of Object?
> Allow ThreadPool used by ThreadPoolFilter to be pluggable
> ---------------------------------------------------------
>
> Key: DIRMINA-184
> URL: http://issues.apache.org/jira/browse/DIRMINA-184
> Project: Directory MINA
> Type: Improvement
> Reporter: peter royal
> Assignee: peter royal
> Fix For: 0.9.5
> Attachments: NewThreadPoolFilter.java, pluggable thread pool.diff
>
> Attached is the start of a patch to allow the ThreadPool that a ThreadPoolFilter uses to be pluggable. Currently depends on Java5, but works as an illustration of thoughts.
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
http://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see:
http://www.atlassian.com/software/jira