Re: svn commit: r1653550 - in /tomcat/trunk/java/org/apache/catalina/ha/deploy: LocalStrings.properties WarWatcher.java
Mark, On 1/21/15 10:37 AM, Mark Thomas wrote: > On 21/01/2015 15:07, schu...@apache.org wrote: >> Author: schultz >> Date: Wed Jan 21 15:07:12 2015 >> New Revision: 1653550 >> >> URL: http://svn.apache.org/r1653550 >> Log: >> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57473 >> Add more logging to WarWatcher, specifically checking for odd states like >> non-existent files that were just listed by the filesystem, which suggests a >> permissions problem. >> Moved log strings into LocalStrings.properties >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties >> tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java >> >> Modified: >> tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties?rev=1653550&r1=1653549&r2=1653550&view=diff >> == >> --- tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties >> Wed Jan 21 15:07:12 2015 >> @@ -45,3 +45,9 @@ farmWarDeployer.stopped=Cluster FarmWarD >> farmWarDeployer.undeployEnd=Undeployment from [{0}] finished. >> farmWarDeployer.undeployLocal=Undeploy local context [{0}] >> farmWarDeployer.watchDir=Cluster deployment is watching [{0}] for changes. >> + >> +warWatcher.checking-wars=Checking WARs in {0} >> +warWatcher.listed-file-does-not-exist={0} was detected in {1} but does not >> exist. Check directory permissions on {1}? >> +warWatcher.checking-war=Checking WAR file {0} >> +warWatcher.check-war.result=WarInfo.check() returned {0} for {1} >> +warWatcher.cant-list-watchDir=Cannot list files in WatchDir "{0}": check to >> see if it is a directory and has read permissions. > > It would have been better to stick to CamelCase format for property > names as in the vast majority of these files - including elsewhere in > this one. Okay, I can change those. >> Modified: tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java >> URL: >> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java?rev=1653550&r1=1653549&r2=1653550&view=diff >> == >> --- tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java >> (original) >> +++ tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java Wed Jan >> 21 15:07:12 2015 >> @@ -23,6 +23,7 @@ import java.util.Map; >> >> import org.apache.juli.logging.Log; >> import org.apache.juli.logging.LogFactory; >> +import org.apache.tomcat.util.res.StringManager; >> >> /** >> * The WarWatcher watches the deployDir for changes made to the >> @@ -36,6 +37,8 @@ public class WarWatcher { >> >> /*--Static Variables*/ >> private static final Log log = LogFactory.getLog(WarWatcher.class); >> +private static final StringManager sm = >> +StringManager.getManager(Constants.Package); > > Coding style is now for up 100 chars in a code line. You probably don't > need to wrap here. ACK > Note there is the option to use the class name directly in getManager(), > removing the need for the package name constant (that usually causes > issues when refactoring in IDEs). Okay. Honestly, when I added the StringManager to the class I was a little iffy about the use of Constants.Package, but that's the way it's done in FarmWarDeployer in the same package, so I decided to leave it. Should we completely remove o.a.c.ha.deploy.Constants entirely, then? It's such a small package, it wouldn't be a big change to remove that one. >> /*--Instance Variables--*/ >> /** >> @@ -67,20 +70,31 @@ public class WarWatcher { >> */ >> public void check() { >> if (log.isDebugEnabled()) >> -log.debug("check cluster wars at " + watchDir); >> +log.debug(sm.getString("warWatcher.checking-wars", watchDir)); >> File[] list = watchDir.listFiles(new WarFilter()); >> -if (list == null) >> +if (list == null) { >> +log.warn(sm.getString("warWatcher.cant-list-watchDir", >> + watchDir)); > > Probably no need to wrap here either. Maybe a few other places below but > I can't tell just by looking at the code - it looks to be close to 100 > in a few places. I usually don't aggressively wrap to 80 columns, but when method calls have tons of arguments in them, I tend to wrap mostly to avoid endless scrolling to see the end of the line. Looks like my default Eclipse view can see out to 102 columns without scrolling, so I'll use that at my metric in the future. >> + >> list = new File[0]; >> +} > > I was wondering about the usefuln
Re: svn commit: r1653550 - in /tomcat/trunk/java/org/apache/catalina/ha/deploy: LocalStrings.properties WarWatcher.java
On 21/01/2015 15:07, schu...@apache.org wrote: > Author: schultz > Date: Wed Jan 21 15:07:12 2015 > New Revision: 1653550 > > URL: http://svn.apache.org/r1653550 > Log: > Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57473 > Add more logging to WarWatcher, specifically checking for odd states like > non-existent files that were just listed by the filesystem, which suggests a > permissions problem. > Moved log strings into LocalStrings.properties > > Modified: > tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties > tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java > > Modified: > tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties?rev=1653550&r1=1653549&r2=1653550&view=diff > == > --- tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties > (original) > +++ tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties > Wed Jan 21 15:07:12 2015 > @@ -45,3 +45,9 @@ farmWarDeployer.stopped=Cluster FarmWarD > farmWarDeployer.undeployEnd=Undeployment from [{0}] finished. > farmWarDeployer.undeployLocal=Undeploy local context [{0}] > farmWarDeployer.watchDir=Cluster deployment is watching [{0}] for changes. > + > +warWatcher.checking-wars=Checking WARs in {0} > +warWatcher.listed-file-does-not-exist={0} was detected in {1} but does not > exist. Check directory permissions on {1}? > +warWatcher.checking-war=Checking WAR file {0} > +warWatcher.check-war.result=WarInfo.check() returned {0} for {1} > +warWatcher.cant-list-watchDir=Cannot list files in WatchDir "{0}": check to > see if it is a directory and has read permissions. It would have been better to stick to CamelCase format for property names as in the vast majority of these files - including elsewhere in this one. > > Modified: tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java > URL: > http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java?rev=1653550&r1=1653549&r2=1653550&view=diff > == > --- tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java (original) > +++ tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java Wed Jan > 21 15:07:12 2015 > @@ -23,6 +23,7 @@ import java.util.Map; > > import org.apache.juli.logging.Log; > import org.apache.juli.logging.LogFactory; > +import org.apache.tomcat.util.res.StringManager; > > /** > * The WarWatcher watches the deployDir for changes made to the > @@ -36,6 +37,8 @@ public class WarWatcher { > > /*--Static Variables*/ > private static final Log log = LogFactory.getLog(WarWatcher.class); > +private static final StringManager sm = > +StringManager.getManager(Constants.Package); Coding style is now for up 100 chars in a code line. You probably don't need to wrap here. Note there is the option to use the class name directly in getManager(), removing the need for the package name constant (that usually causes issues when refactoring in IDEs). > > /*--Instance Variables--*/ > /** > @@ -67,20 +70,31 @@ public class WarWatcher { > */ > public void check() { > if (log.isDebugEnabled()) > -log.debug("check cluster wars at " + watchDir); > +log.debug(sm.getString("warWatcher.checking-wars", watchDir)); > File[] list = watchDir.listFiles(new WarFilter()); > -if (list == null) > +if (list == null) { > +log.warn(sm.getString("warWatcher.cant-list-watchDir", > + watchDir)); Probably no need to wrap here either. Maybe a few other places below but I can't tell just by looking at the code - it looks to be close to 100 in a few places. > + > list = new File[0]; > +} I was wondering about the usefulness of a debug message here saying how many files were found. Not sure though. Mark > //first make sure all the files are listed in our current status > for (int i = 0; i < list.length; i++) { > +if(!list[i].exists()) > + > log.warn(sm.getString("warWatcher.listed-file-does-not-exist", > + list[i], watchDir)); > + > addWarInfo(list[i]); > } > > -//check all the status codes and update the FarmDeployer > +// Check all the status codes and update the FarmDeployer > for (Iterator> i = > currentStatus.entrySet().iterator(); i.hasNext();) { > Map.Entry entry = i.next(); > WarInfo info = entry.getValue(); > +if(log.isTraceEnabled()) > +log
svn commit: r1653550 - in /tomcat/trunk/java/org/apache/catalina/ha/deploy: LocalStrings.properties WarWatcher.java
Author: schultz Date: Wed Jan 21 15:07:12 2015 New Revision: 1653550 URL: http://svn.apache.org/r1653550 Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=57473 Add more logging to WarWatcher, specifically checking for odd states like non-existent files that were just listed by the filesystem, which suggests a permissions problem. Moved log strings into LocalStrings.properties Modified: tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java Modified: tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties?rev=1653550&r1=1653549&r2=1653550&view=diff == --- tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties (original) +++ tomcat/trunk/java/org/apache/catalina/ha/deploy/LocalStrings.properties Wed Jan 21 15:07:12 2015 @@ -45,3 +45,9 @@ farmWarDeployer.stopped=Cluster FarmWarD farmWarDeployer.undeployEnd=Undeployment from [{0}] finished. farmWarDeployer.undeployLocal=Undeploy local context [{0}] farmWarDeployer.watchDir=Cluster deployment is watching [{0}] for changes. + +warWatcher.checking-wars=Checking WARs in {0} +warWatcher.listed-file-does-not-exist={0} was detected in {1} but does not exist. Check directory permissions on {1}? +warWatcher.checking-war=Checking WAR file {0} +warWatcher.check-war.result=WarInfo.check() returned {0} for {1} +warWatcher.cant-list-watchDir=Cannot list files in WatchDir "{0}": check to see if it is a directory and has read permissions. Modified: tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java?rev=1653550&r1=1653549&r2=1653550&view=diff == --- tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java (original) +++ tomcat/trunk/java/org/apache/catalina/ha/deploy/WarWatcher.java Wed Jan 21 15:07:12 2015 @@ -23,6 +23,7 @@ import java.util.Map; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.res.StringManager; /** * The WarWatcher watches the deployDir for changes made to the @@ -36,6 +37,8 @@ public class WarWatcher { /*--Static Variables*/ private static final Log log = LogFactory.getLog(WarWatcher.class); +private static final StringManager sm = +StringManager.getManager(Constants.Package); /*--Instance Variables--*/ /** @@ -67,20 +70,31 @@ public class WarWatcher { */ public void check() { if (log.isDebugEnabled()) -log.debug("check cluster wars at " + watchDir); +log.debug(sm.getString("warWatcher.checking-wars", watchDir)); File[] list = watchDir.listFiles(new WarFilter()); -if (list == null) +if (list == null) { +log.warn(sm.getString("warWatcher.cant-list-watchDir", + watchDir)); + list = new File[0]; +} //first make sure all the files are listed in our current status for (int i = 0; i < list.length; i++) { +if(!list[i].exists()) +log.warn(sm.getString("warWatcher.listed-file-does-not-exist", + list[i], watchDir)); + addWarInfo(list[i]); } -//check all the status codes and update the FarmDeployer +// Check all the status codes and update the FarmDeployer for (Iterator> i = currentStatus.entrySet().iterator(); i.hasNext();) { Map.Entry entry = i.next(); WarInfo info = entry.getValue(); +if(log.isTraceEnabled()) +log.trace(sm.getString("warWatcher.checking-war", + info.getWar())); int check = info.check(); if (check == 1) { listener.fileModified(info.getWar()); @@ -89,6 +103,10 @@ public class WarWatcher { //no need to keep in memory i.remove(); } +if(log.isTraceEnabled()) +log.trace(sm.getString("warWatcher.check-war.result", + Integer.valueOf(check), + info.getWar())); } } - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org