Bug 15599 - SocketAppender ignores ReconnectionDelay of 0 (with fix)
SocketAppender ignores ReconnectionDelay of 0 (with fix)
Status: RESOLVED FIXED
Product: Log4j
Classification: Unclassified
Component: Appender
1.2
All All
: P3 normal
: ---
Assigned To: log4j-dev
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2002-12-21 22:49 UTC by Scott Schram
Modified: 2004-11-16 19:05 UTC (History)
0 users



Attachments
Proposed fix. Lemme know if I did this patch right. (1.01 KB, patch)
2002-12-21 23:00 UTC, Scott Schram
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Scott Schram 2002-12-21 22:49:17 UTC
If you set the ReconnectionDelay of a SocketAppender to zero, it is supposed to
not retry when it fails to connect.

If it successfully connects the first time, it honors the zero setting (with a
test in SocketAppender.java append(LoggingEvent event)).

However, if it fails to connect the first time, it will try to connect an
infinite number of times, with no delay between tries.

The reason is that there's no test for a zero setting in SocketAppender.java
connect() before calling fireConnector().

I have a proposed fix for this bug, and it includes two minor enhancements:

1) If ReconnectionDelay is zero, the message won't say that it "will try again
later'.

2) If ReconnectionDelay is zero, it does not output the lengthy exception that
you get if it can't connect.

The rationale for this second improvement is that we use ReconnectionDelay of
zero when we are testing on our development machines.  If we are running
chainsaw, we want to see the logging info in chainsaw.  However, if chainsaw
isn't running, we'd like it to fail with a brief message instead of the lengthy
exception to the console.

Setting ReconnectionDelay of zero strongly implies that "it's not that
important" if it gets a connection, or tries to bring it back up, so I don't
think I'm creating too much controversy with that change.  Further, there are
probably not too many people setting it to zero because nobody reported this bug
yet. :)

Here's my proposed replacement:

  void connect(InetAddress address, int port) {
    if(this.address == null)
      return;
    try {
      // First, close the previous connection if any.
      cleanUp();
      oos = new ObjectOutputStream(new Socket(address, port).getOutputStream());
    } catch(IOException e) {
      String msg = "Could not connect to remote log4j server at ["
		   +address.getHostName()+"].";

      if(reconnectionDelay > 0) {
        msg += " We will try again later.";
        LogLog.error(msg, e);
      }
      else {
        LogLog.error(msg);
      }
          
      if(reconnectionDelay > 0) {      
        fireConnector();
      }
    }
  }
Comment 1 Scott Schram 2002-12-21 23:00:40 UTC
Created attachment 4250 [details]
Proposed fix.  Lemme know if I did this patch right.
Comment 2 Scott Schram 2002-12-22 06:02:53 UTC
I see we could move fireConnector(); into the first

if(reconnectionDelay > 0)

right after

LogLog.error(msg, e);

and eliminate the second test.
Comment 3 Mark Womack 2003-01-03 21:34:07 UTC
I'll take a look at applying this change.
Comment 4 Ceki Gulcu 2003-02-18 00:54:11 UTC
Thanks for reporting this bug. I applied your patch, except that the stack 
trace is still reported. Altough you might not be interesed in the stack trace 
other users might be. It is hard to tell. I thus prefer to err on the safe 
side.