You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by "Peter M. Goldstein" <pe...@yahoo.com> on 2002/11/12 08:33:07 UTC

[PATCH/VOTE] - POP3 Performance/Functionality Fix.

All,

This is a fix for the observed performance problem with POP3 on some
platform JVM combinations.  Basically it amounts to adding a
BufferedOutputStream around the OutputStream produced by the socket.
This also seems to resolve this issue encountered by JRC (Randy),
although the reason for this is less clear.

As we are past code freeze, this requires a vote of the committers to be
accepted.  I think the problem is severe enough (as it is impacting our
user community) and the fix is localized enough to merit a patch, so I
vote +1.  Thoughts?

--Peter

Re: [PATCH/VOTE] - POP3 Performance/Functionality Fix.

Posted by simon <si...@sydneybluegum.com>.
Peter,   Thank you.

            I have downloaded the latest cvs and added your changes and 
my test message of 1.5meg is now
downloaded in 3 or 4 seconds compared to 3 minutes and 46 seconds.
My system is Java 1.4.1 on windows 2000. Will move to linux soon.

Had posted a message on the users email list and Noel was going to look 
into it.
Would be good for all those windows people having the same problem if 
this fix could be included in the next release.

Please post the solution on the on the users list when available.

Thanks again.
Regards,
Simon


Peter M. Goldstein wrote:

>All,
>
>This is a fix for the observed performance problem with POP3 on some
>platform JVM combinations.  Basically it amounts to adding a
>BufferedOutputStream around the OutputStream produced by the socket.
>This also seems to resolve this issue encountered by JRC (Randy),
>although the reason for this is less clear.
>
>As we are past code freeze, this requires a vote of the committers to be
>accepted.  I think the problem is severe enough (as it is impacting our
>user community) and the fix is localized enough to merit a patch, so I
>vote +1.  Thoughts?
>
>--Peter
>  
>
>------------------------------------------------------------------------
>
>Index: jakarta-james/src/java/org/apache/james/pop3server/POP3Handler.java
>===================================================================
>RCS file: /home/cvs/jakarta-james/src/java/org/apache/james/pop3server/POP3Handler.java,v
>retrieving revision 1.17
>diff -u -r1.17 POP3Handler.java
>--- jakarta-james/src/java/org/apache/james/pop3server/POP3Handler.java	2 Nov 2002 09:03:52 -0000	1.17
>+++ jakarta-james/src/java/org/apache/james/pop3server/POP3Handler.java	12 Nov 2002 07:24:45 -0000
>@@ -238,7 +238,7 @@
>         }
> 
>         try {
>-            outs = socket.getOutputStream();
>+            outs = new BufferedOutputStream(socket.getOutputStream(), 1024);
>             out = new InternetPrintWriter(outs, true);
>             state = AUTHENTICATION_READY;
>             user = "unknown";
>@@ -846,8 +846,6 @@
>                 writeLoggedFlushedResponse(responseString);
>                 return;
>             }
>-            //?May be written as
>-            //return parseCommand("TOP " + num + " " + Integer.MAX_VALUE);?
>             try {
>                 MailImpl mc = (MailImpl) userMailbox.elementAt(num);
>                 if (mc != DELETED) {
>@@ -859,6 +857,8 @@
>                                                               theWatchdog,
>                                                               theConfigData.getResetLength());
>                     mc.writeMessageTo(nouts);
>+                    nouts.flush();
>+                    // TODO: Is this an extra CRLF?
>                     out.println();
>                     out.println(".");
>                     out.flush();
>@@ -888,7 +888,6 @@
>                 responseString = responseBuffer.toString();
>                 writeLoggedFlushedResponse(responseString);
>             }
>-            // -------------------------------------------?
>         } else {
>             responseString = ERR_RESPONSE;
>             writeLoggedFlushedResponse(responseString);
>@@ -935,6 +934,7 @@
>                                                               theWatchdog,
>                                                               theConfigData.getResetLength());
>                     mc.writeContentTo(nouts, lines);
>+                    nouts.flush();
>                     out.println(".");
>                     out.flush();
>                 } else {
>
>  
>
>------------------------------------------------------------------------
>
>--
>To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
>For additional commands, e-mail: <ma...@jakarta.apache.org>
>



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: [PATCH/VOTE] - POP3 Performance/Functionality Fix.

Posted by "Peter M. Goldstein" <pe...@yahoo.com>.
Serge,

> +1 as long as you've got flush's in all the right places.

Yep.  We use a BufferedReader everywhere else, so the flushes were
already in place (see some of the late stage bug fixes on POP3 for this
revision).  It's only in doTOP and doRETR that we directly access the
underlying OutputStream, and thus only there that additional flush()
calls were required.

--Peter



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [PATCH/VOTE] - POP3 Performance/Functionality Fix.

Posted by Serge Knystautas <se...@lokitech.com>.
Peter M. Goldstein wrote:
> All,
> 
> This is a fix for the observed performance problem with POP3 on some
> platform JVM combinations.  Basically it amounts to adding a
> BufferedOutputStream around the OutputStream produced by the socket.
> This also seems to resolve this issue encountered by JRC (Randy),
> although the reason for this is less clear.
> 
> As we are past code freeze, this requires a vote of the committers to be
> accepted.  I think the problem is severe enough (as it is impacting our
> user community) and the fix is localized enough to merit a patch, so I
> vote +1.  Thoughts?

+1 as long as you've got flush's in all the right places.

-- 
Serge Knystautas
Loki Technologies
http://www.lokitech.com/


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: [PATCH/VOTE] - POP3 Performance/Functionality Fix.

Posted by "Noel J. Bergman" <no...@devtech.com>.
> I accept your logic but I'm extremely wary of introducing bugs at this
stage.
> Any more late changes will get a -0 or even -1 from me on this basis
alone.

I understand, Danny.  In this case, the latter patch fixed a bunch of user
complaints.  The MX patch could perhaps have been deferred.

> I was under the impression (I may be wrong) that MX priorities exist only
> in order toprovide fail-over and basic load sharing.

Isn't that what Peter's patch did?  Provide fail-over?

	--- Noel


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


RE: [PATCH/VOTE] - POP3 Performance/Functionality Fix.

Posted by Danny Angus <da...@apache.org>.
 +0 
To this and peter's previous patch.

I accept your logic but I'm extremely wary of introducing bugs at this stage.
Any more late changes will get a -0 or even -1 from me on this basis alone.

In particular the MX patch could, although I trust you to have got it right, easily lead to unforseen loops or recursions especially as here we are making decisions base on a set of possible responses which we cannot be 100% sure we have fully enumerated. Even if we have identified every possible legal response we may still find illegal responses.

For what its worth I think that handling mail server routing by terminating connections after they have been made and relying on the sender to try another host until one accepts the mail is a pretty poor hack in the first place when compared with using alternatives, like packet filtering to route the connection to the correct host in the first instance, and good old mail forwarding by rules.
I was under the impression (I may be wrong) that MX priorities exist only in order to provide fail-over and basic load sharing.


d.

 
> As we are past code freeze, this requires a vote of the committers to be
> accepted.  I think the problem is severe enough (as it is impacting our
> user community) and the fix is localized enough to merit a patch, so I
> vote +1.  Thoughts?


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>