You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by James Im <im...@hotmail.com> on 2007/07/28 12:07:30 UTC
Rationale for Thread.sleep(1000) in Worker ?
I'm looking a little bit at the source code today and there is something
that I don't understand.
In the class SocketAcceptor.Worker, in the catch block there is a
"Thread.sleep(1000);". I don't understand why you do that. What problem
does this solve? Could you add a comment in the source that tells why
this sleep is needed? That would be the best place to document this as
its purpose is not obvious. Should I create a jira issue for that?
} catch (IOException e) {
ExceptionMonitor.getInstance().exceptionCaught(e);
try {
Thread.sleep(1000);
} catch (InterruptedException e1) {
ExceptionMonitor.getInstance().exceptionCaught(e1);
}
}
_________________________________________________________________
Ta' pĺ udsalg ĺret rundt pĺ MSN Shopping: http://shopping.msn.dk - her
finder du altid de bedste priser
Re: Rationale for Thread.sleep(1000) in Worker ?
Posted by Trustin Lee <tr...@gmail.com>.
On 7/28/07, James Im <im...@hotmail.com> wrote:
> I'm looking a little bit at the source code today and there is something
> that I don't understand.
>
> In the class SocketAcceptor.Worker, in the catch block there is a
> "Thread.sleep(1000);". I don't understand why you do that. What problem
> does this solve? Could you add a comment in the source that tells why
> this sleep is needed? That would be the best place to document this as
> its purpose is not obvious. Should I create a jira issue for that?
>
> } catch (IOException e) {
> ExceptionMonitor.getInstance().exceptionCaught(e);
>
> try {
> Thread.sleep(1000);
> } catch (InterruptedException e1) {
> ExceptionMonitor.getInstance().exceptionCaught(e1);
> }
> }
The sleep here is inserted for the case that an IOException is kept
thrown when a network card goes down. Once the network card is
disabled by 'ifdown', Selector can throw an IOException infinitely
until it's enabled again. I added sleep(1000); to avoid excessive
exception logging.
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
Re: Rationale for Thread.sleep(1000) in Worker ?
Posted by Trustin Lee <tr...@gmail.com>.
On 7/29/07, Mark <el...@gmail.com> wrote:
> I can add the comments into the code, but I looked at the SocketIoProcessor
> class in the trunk, and the code is a little different. There is no call to
> selector.close(). Approx line 510.
It's OK. We will call selector.close() as soon as we get a Selector
pool. Please add the comment to all branches.
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
Re: Rationale for Thread.sleep(1000) in Worker ?
Posted by Mark <el...@gmail.com>.
I can add the comments into the code, but I looked at the SocketIoProcessor
class in the trunk, and the code is a little different. There is no call to
selector.close(). Approx line 510.
On 7/28/07, Trustin Lee <tr...@gmail.com> wrote:
>
> On 7/29/07, Adam Fisk <a...@lastbamboo.org> wrote:
> > I agree that would be very helpful. A comment in that same method for
> this
> > chunk of code would also be very useful:
> >
> > if (selector.keys().isEmpty()) {
> > synchronized (lock) {
> > if (selector.keys().isEmpty()
> > && newSessions.isEmpty()) {
> > worker = null;
> >
> > try {
> > selector.close();
> > } catch (IOException e) {
> > ExceptionMonitor.getInstance()
> > .exceptionCaught(e);
> > } finally {
> > selector = null;
> > }
> >
> > break;
> > }
> >
> > This relates to the question of visibility and possible use of volatile
> you
> > mentioned earlier. Why are we stopping the selector here, only to
> seemingly
> > start it again when new sessions come in? A comment would be great! If
> > anyone's worried about their English, just an explanation on the list
> for
> > the rationale would be fantastic, and I'd be happy to submit a patch
> with
> > more fully commented code.
>
> The code block above is for stopping the SocketIoProcessor thread
> automatically when there's no sessions to manage. By doing so, we can
> maintain the number of I/O processor therads in an optimal condition,
> and avoid the situation that JVM doesn't exit because of idle I/O
> processor threads.
>
> HTH,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/
> --
> PGP Key ID: 0x0255ECA6
>
--
..Cheers
Mark
Re: Rationale for Thread.sleep(1000) in Worker ?
Posted by Trustin Lee <tr...@gmail.com>.
On 7/29/07, Adam Fisk <a...@lastbamboo.org> wrote:
> I agree that would be very helpful. A comment in that same method for this
> chunk of code would also be very useful:
>
> if (selector.keys().isEmpty()) {
> synchronized (lock) {
> if (selector.keys().isEmpty()
> && newSessions.isEmpty()) {
> worker = null;
>
> try {
> selector.close();
> } catch (IOException e) {
> ExceptionMonitor.getInstance()
> .exceptionCaught(e);
> } finally {
> selector = null;
> }
>
> break;
> }
>
> This relates to the question of visibility and possible use of volatile you
> mentioned earlier. Why are we stopping the selector here, only to seemingly
> start it again when new sessions come in? A comment would be great! If
> anyone's worried about their English, just an explanation on the list for
> the rationale would be fantastic, and I'd be happy to submit a patch with
> more fully commented code.
The code block above is for stopping the SocketIoProcessor thread
automatically when there's no sessions to manage. By doing so, we can
maintain the number of I/O processor therads in an optimal condition,
and avoid the situation that JVM doesn't exit because of idle I/O
processor threads.
HTH,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/
--
PGP Key ID: 0x0255ECA6
Re: Rationale for Thread.sleep(1000) in Worker ?
Posted by Adam Fisk <a...@lastbamboo.org>.
I agree that would be very helpful. A comment in that same method for this
chunk of code would also be very useful:
if (selector.keys().isEmpty()) {
synchronized (lock) {
if (selector.keys().isEmpty()
&& newSessions.isEmpty()) {
worker = null;
try {
selector.close();
} catch (IOException e) {
ExceptionMonitor.getInstance()
.exceptionCaught(e);
} finally {
selector = null;
}
break;
}
This relates to the question of visibility and possible use of volatile you
mentioned earlier. Why are we stopping the selector here, only to seemingly
start it again when new sessions come in? A comment would be great! If
anyone's worried about their English, just an explanation on the list for
the rationale would be fantastic, and I'd be happy to submit a patch with
more fully commented code.
-Adam
On 7/28/07, James Im <im...@hotmail.com> wrote:
>
> I'm looking a little bit at the source code today and there is something
> that I don't understand.
>
> In the class SocketAcceptor.Worker, in the catch block there is a
> "Thread.sleep(1000);". I don't understand why you do that. What problem
> does this solve? Could you add a comment in the source that tells why
> this sleep is needed? That would be the best place to document this as
> its purpose is not obvious. Should I create a jira issue for that?
>
> } catch (IOException e) {
> ExceptionMonitor.getInstance().exceptionCaught(e);
>
> try {
> Thread.sleep(1000);
> } catch (InterruptedException e1) {
> ExceptionMonitor.getInstance().exceptionCaught(e1);
> }
> }
>
> _________________________________________________________________
> Ta' på udsalg året rundt på MSN Shopping: http://shopping.msn.dk - her
> finder du altid de bedste priser
>
>