You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by sc...@apache.org on 2015/01/21 16:07:12 UTC
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 <b>WarWatcher </b> 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<Map.Entry<String,WarInfo>> i =
currentStatus.entrySet().iterator(); i.hasNext();) {
Map.Entry<String,WarInfo> 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1653550 - in
/tomcat/trunk/java/org/apache/catalina/ha/deploy: LocalStrings.properties
WarWatcher.java
Posted by Christopher Schultz <ch...@christopherschultz.net>.
Mark,
On 1/21/15 10:37 AM, Mark Thomas wrote:
> On 21/01/2015 15:07, schultz@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 <b>WarWatcher </b> 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 usefulness of a debug message here saying how
> many files were found. Not sure though.
With trace logging enabled, you'll see the name of every WAR file
processed. If you don't see any trace logs, then we found no files.
I'd never object to more trace- or debug-level logging, though.
Thanks,
-chris
Re: svn commit: r1653550 - in /tomcat/trunk/java/org/apache/catalina/ha/deploy:
LocalStrings.properties WarWatcher.java
Posted by Mark Thomas <ma...@apache.org>.
On 21/01/2015 15:07, schultz@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 <b>WarWatcher </b> 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<Map.Entry<String,WarInfo>> i =
> currentStatus.entrySet().iterator(); i.hasNext();) {
> Map.Entry<String,WarInfo> 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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org