admin@glassfish.java.net

Re: CODE REVIEW (LogMBean): minor bug fixes for 9.1.1 AMX unit test failures

From: Kedar Mhaswade <Kedar.Mhaswade_at_Sun.COM>
Date: Tue, 04 Dec 2007 14:55:34 -0800

Lloyd L Chambers wrote:
> Diffs for fix at the source.

Thank you, for doing it. I *really* appreciate that. It was not fine
masking it in AMX.

>
> - had to make the prefix and suffix of the filename available so log
> files that match can be found. Now only files that start with "server"
> and end with ".log" will be returned as log file names.

Ummm. That probably works. But how about returning all the files except
jvm.log. But that's just a thought. If this works, I am OK.

>
> - made a few things 'final'. Removed the unnecessary 'synchronized'
> from getInstance(). Maybe I should not do this, what do you think?
> Logging is heavily used. getInstance() can only return the same thing,
> forever.

Wow. I am not sure. One part of me says, yes, we "must" do what you have
done. It does not make sense to make a method synchronized when it just returns
a final variable that's initialized along with declaration. Good observation!

But the other part of me says -- Do you really want to take the responsibility?
Maybe not. Who knows what'll break because of that.

I think the best way to address this is to create a separate bug and
assign it to "logging" category. That way, it can be analyzed better.

- Kedar

>
> - FilenameFilterImpl was a separate file (pointless). Moved it into
> LogMBean.
>
> dhcp-usca14-133-62:/gf/build/glassfish/appserv-core lloyd$ cvs diff -uw
> src/java/com/sun/enterprise/server/logging
> ? src/java/com/sun/enterprise/server/logging/.DS_Store
> cvs diff: Diffing src/java/com/sun/enterprise/server/logging
> Index: src/java/com/sun/enterprise/server/logging/FileandSyslogHandler.java
> ===================================================================
> RCS file:
> /cvs/glassfish/appserv-core/src/java/com/sun/enterprise/server/logging/FileandSyslogHandler.java,v
>
> retrieving revision 1.10
> diff -u -w -r1.10 FileandSyslogHandler.java
> ---
> src/java/com/sun/enterprise/server/logging/FileandSyslogHandler.java
> 5 May 2007 05:35:43 -0000 1.10
> +++
> src/java/com/sun/enterprise/server/logging/FileandSyslogHandler.java
> 4 Dec 2007 22:41:37 -0000
> @@ -84,7 +84,11 @@
>
> private Date date = new Date( );
> private static final String LOGS_DIR = "logs";
> - private String logFileName = "server.log";
> +
> + static final String LOG_FILENAME_PREFIX = "server";
> + static final String LOG_FILENAME_SUFFIX = ".log";
> +
> + private static final String logFileName = LOG_FILENAME_PREFIX +
> LOG_FILENAME_SUFFIX;
>
> private String absoluteFileName = null;
>
> @@ -109,14 +113,14 @@
> // Initially the LogRotation will be off until the domain.xml value
> is read.
> private int limitForFileRotation = 0;
>
> - private Object fileUpdateLock = new Object( );
> + private final Object fileUpdateLock = new Object( );
>
> private boolean rotationInProgress = false;
>
> private static final FileandSyslogHandler thisInstance =
> new FileandSyslogHandler( );
>
> - private LinkedList pendingLogRecordList = new LinkedList();
> + private final LinkedList pendingLogRecordList = new LinkedList();
>
> // Rotation can be done in 3 ways
> // 1. Based on the Size: Rotate when some Threshold number of bytes
> are
> @@ -127,7 +131,7 @@
> // be fired from the publish( ) method for consistency
> private static boolean rotationRequested = false;
>
> - public static synchronized FileandSyslogHandler getInstance( ) {
> + public static FileandSyslogHandler getInstance( ) {
> return thisInstance;
> }
>
> cvs diff:
> src/java/com/sun/enterprise/server/logging/FilenameFilterImpl.java was
> removed, no comparison available
> Index: src/java/com/sun/enterprise/server/logging/LogMBean.java
> ===================================================================
> RCS file:
> /cvs/glassfish/appserv-core/src/java/com/sun/enterprise/server/logging/LogMBean.java,v
>
> retrieving revision 1.14
> diff -u -w -r1.14 LogMBean.java
> --- src/java/com/sun/enterprise/server/logging/LogMBean.java 5 May
> 2007 05:35:44 -0000 1.14
> +++ src/java/com/sun/enterprise/server/logging/LogMBean.java 4 Dec
> 2007 22:41:38 -0000
> @@ -693,6 +693,21 @@
> listOfModules, nameValueMap);
> }
>
> +
> + private static final class FilenameFilterImpl implements
> java.io.FilenameFilter {
> + public boolean accept( File dir, String fileName ) {
> + boolean include = false;
> +
> + // exclude any other log files except for server*.log
> + if( fileName.endsWith(
> FileandSyslogHandler.LOG_FILENAME_SUFFIX ) &&
> + fileName.startsWith(
> FileandSyslogHandler.LOG_FILENAME_PREFIX ) ) {
> + include = true;
> + }
> +
> + return include;
> + }
> + }
> +
> /**
> * Gets all Archived log Files in the current Server instance.
> *
>
>
>
>
> On Dec 3, 2007, at 10:30 AM, Kedar Mhaswade wrote:
>
>>> I'll look into fixing the log problem at the source.
>>
>> Thank you.
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
> ---
> Lloyd L Chambers
> lloyd.chambers_at_sun.com
> Sun Microsystems, Inc
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>