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 no...@apache.org on 2006/11/04 19:28:54 UTC
svn commit: r471243 -
/james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Author: noel
Date: Sat Nov 4 10:28:53 2006
New Revision: 471243
URL: http://svn.apache.org/viewvc?view=rev&rev=471243
Log:
JAMES-671. Don't accept() unless we're prepared to process the connection. This change is perhaps excessively conservative. We can probably eliminate the loop and the timeout value for wait (which is just a backstop), and just rely upon notification.
Modified:
james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Modified: james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
URL: http://svn.apache.org/viewvc/james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java?view=diff&rev=471243&r1=471242&r2=471243
==============================================================================
--- james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java (original)
+++ james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java Sat Nov 4 10:28:53 2006
@@ -263,6 +263,8 @@
runnerPool.put(clientConnectionRunner);
}
}
+
+ synchronized (this) { notify(); } // match the wait(...) in the run() inner loop before accept().
}
/**
@@ -343,7 +345,13 @@
try {
Socket clientSocket = null;
try {
+ while (maxOpenConn > 0 && clientConnectionRunners.size() >= maxOpenConn) {
+ getLogger().warn("Maximum number of open connections (" + clientConnectionRunners.size() + ") in use.");
+ synchronized (this) { wait(10000); }
+ }
+
clientSocket = serverSocket.accept();
+
} catch( InterruptedIOException iioe ) {
// This exception is expected upon ServerConnection shutdown.
// See the POLLING_INTERVAL comment
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by "Noel J. Bergman" <no...@devtech.com>.
> please have a look at target\test\reports for unit test reports.
Thanks. I'll look when I land.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by Bernd Fondermann <be...@googlemail.com>.
Hi Noel,
please have a look at
target\test\reports
for unit test reports.
Bernd
On 11/6/06, Noel J. Bergman <no...@devtech.com> wrote:
> > I've not checked in detail, but it seems that this patch broke our unit
> > tests
>
> Yes, it seems that org.apache.james.smtpserver.SMTPServerTest is failing. I don't have time to look into why before I leave for the airport. As you say, it could either be a test assumption or a defect in the fix.
>
> I'll look later tonight if no one else gets to it first.
>
> It would be nice if the tests would tell us something more than "failed." If the test is saying what went wrong, I'm not spotting it.
>
> ref: http://mail-archives.apache.org/mod_mbox/james-server-dev/200611.mbox/%3c18536904.52191162802383496.JavaMail.root@devtech.com%3e
>
> http://mail-archives.apache.org/mod_mbox/james-server-dev/200611.mbox/%3c3579293.25231162456448491.JavaMail.root@devtech.com%3e
>
> The only difference I see is Worker #3.
>
> --- Noel
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
> For additional commands, e-mail: server-dev-help@james.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by Bernd Fondermann <be...@googlemail.com>.
On 11/7/06, Noel J. Bergman <no...@devtech.com> wrote:
> Stefano Bagnara wrote:
>
> > The second connection made (using a connection limit of 1) expect
> > to received a connection exception, instead it seems to get
> > stalled waiting to connect.
>
> If I understand you (and the code in testConnectionLimitExceeded) correctly, it seems that the defect is in the test's assumption not the ServerConnection code.
The one (Test) can not live without the other (Testee) - and the other
way round, I'd say.
It is best practice to run, extend, adjust and re-run unit tests when
the implementation changes. Otherwise, the tests are pretty useless.
> what needs to be changed is the test, not the revised ServerConnection. :-)
Right. Thanks for volunteering ;-)
Bernd
PS: google feeling lucky for "test infected"
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by "Noel J. Bergman" <no...@devtech.com>.
Stefano Bagnara wrote:
> The second connection made (using a connection limit of 1) expect
> to received a connection exception, instead it seems to get
> stalled waiting to connect.
If I understand you (and the code in testConnectionLimitExceeded) correctly, it seems that the defect is in the test's assumption not the ServerConnection code.
A connection limit of 1 means that the first connection will be handled, and the second connection will be ESTABLISHED in the TCP/IP connection queue. That is the expected and desired behavior. The old behavior was wrong: it constantly purged the backlog queue and discarded connections. The correct behavior processes the queued connection when a worker is available.
Kudos to Berd's code for detecting the change, but what needs to be changed is the test, not the revised ServerConnection. :-)
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by Stefano Bagnara <ap...@bago.org>.
Noel J. Bergman wrote:
>> I've not checked in detail, but it seems that this patch broke our unit
>> tests
>
> Yes, it seems that org.apache.james.smtpserver.SMTPServerTest is failing. I don't have time to look into why before I leave for the airport. As you say, it could either be a test assumption or a defect in the fix.
>
> I'll look later tonight if no one else gets to it first.
>
> It would be nice if the tests would tell us something more than "failed." If the test is saying what went wrong, I'm not spotting it.
>
> ref: http://mail-archives.apache.org/mod_mbox/james-server-dev/200611.mbox/%3c18536904.52191162802383496.JavaMail.root@devtech.com%3e
>
> http://mail-archives.apache.org/mod_mbox/james-server-dev/200611.mbox/%3c3579293.25231162456448491.JavaMail.root@devtech.com%3e
>
> The only difference I see is Worker #3.
>
> --- Noel
Just opened it and ran tests: it seems a problem in the connection limit
test. The second connection made (using a connection limit of 1) expect
to received a connection exception, instead it seems to get stalled
waiting to connect.
I've not investigated on this.
Stefano
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by "Noel J. Bergman" <no...@devtech.com>.
> I've not checked in detail, but it seems that this patch broke our unit
> tests
Yes, it seems that org.apache.james.smtpserver.SMTPServerTest is failing. I don't have time to look into why before I leave for the airport. As you say, it could either be a test assumption or a defect in the fix.
I'll look later tonight if no one else gets to it first.
It would be nice if the tests would tell us something more than "failed." If the test is saying what went wrong, I'm not spotting it.
ref: http://mail-archives.apache.org/mod_mbox/james-server-dev/200611.mbox/%3c18536904.52191162802383496.JavaMail.root@devtech.com%3e
http://mail-archives.apache.org/mod_mbox/james-server-dev/200611.mbox/%3c3579293.25231162456448491.JavaMail.root@devtech.com%3e
The only difference I see is Worker #3.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
Re: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by Stefano Bagnara <ap...@bago.org>.
I've not checked in detail, but it seems that this patch broke our unit
tests (I noticed this in my continuum, but you can see it also in the
Nighly build report).
I already checked that this is not the "ramdom" problem we saw in past:
this seems to happen every time.
Maybe the tests are wrong, or maybe the patch does not work like expected.
Stefano
Noel J. Bergman wrote:
> This is working pretty well in my v2.3.1 build. I can watch connections work their way through the TCP/IP backlog now, and can see the effect of the wait/notify.
>
> --- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org
RE: svn commit: r471243 - /james/server/trunk/src/java/org/apache/james/util/connection/ServerConnection.java
Posted by "Noel J. Bergman" <no...@devtech.com>.
This is working pretty well in my v2.3.1 build. I can watch connections work their way through the TCP/IP backlog now, and can see the effect of the wait/notify.
--- Noel
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org