You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Christopher Schultz <ch...@christopherschultz.net> on 2019/08/06 20:05:46 UTC

Proposal: use Files.move instead of File.renameTo in FarmWarDeployer

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

All,

Someone recently had a problem where the FarmWarDeployer wouldn't work
on a secondary node because File.renameTo was failing -- likely due to
the underlying Java/OS refusing to re-name a file across filesystems.

I propose that we switch to using Files.move which will either re-name
or move depending upon what is necessary. It also throws an exception
if it can't do its work, rather than failing and returning false.

Code patch below. I would also remove all of the
"farmWarDeployer.renameFail" error message keys from the resource bundle
s.

Thanks,
- -chris


diff --git a/java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
b/java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
index cccc25b978..260355681c 100644
- --- a/java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
+++ b/java/org/apache/catalina/ha/deploy/FarmWarDeployer.java
@@ -19,6 +19,7 @@ package org.apache.catalina.ha.deploy;

 import java.io.File;
 import java.io.IOException;
+import java.nio.file.Files;
 import java.util.HashMap;

 import javax.management.MBeanServer;
@@ -232,11 +233,7 @@ public class FarmWarDeployer extends ClusterListene
r
                             addServiced(contextName);
                             try {
                                 remove(contextName);
- -                                if
(!factory.getFile().renameTo(deployable)) {
- -                                    log.error(sm.getString(
- -                                            "farmWarDeployer.renameFail
",
- -                                            factory.getFile(),
deployable));
- -                                }
+
Files.move(factory.getFile().toPath(), deployable.toPath());
                                 check(contextName);
                             } finally {
                                 removeServiced(contextName);
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl1J3ZkACgkQHPApP6U8
pFiDABAAoUi4aNiFOg5xs5qbh2vUVXNn0yhettdRToLNwIFgRNrEhXp19YawjZJ6
n7y64G89T+FlAeuov9E8FGIFFAOnVnOL9a5pOYS4ADGYHpEnHQQP5isqaCfwv8eW
btf0jb33bqjP37IdnKm26ZHNnP61APoZfMtWJ7fkOUjan0satK2LBBkzwYCMeaPS
QFogb2pnlAHiCE15TBF1M91aMfCh8UVotpkSp5SYhFhw5Yd0kdehxrDu49pbxf9B
njeOhMRIL4tsbHG+Bd3+mzk/AGvtpgZcF2cBIir72j+1SYFQZGmN4C1YS0mCl1m7
BP7t6LMMZLVJSzjtFi0MsC/UJThJ7MI79yf19aR0IHN9K/QSlPxVr0xSIm9Q9PlW
pasplFWfG/qWI+QyEgf854fK+sIQrb7ymk45tRvdeyO1f0TC9mamsCi/3d4BQi5j
PBCgqnnoNX7a/xdCi5eavHi3hzjX0O987cZ9mK3K59hj2an5WoY9hkrO4y/7kTpM
sWo8JAcJ2pCJoaHxPf5+bMIf+49lcW4jgaQRsk37m3hf1k89e9aZTDmtYNHULgZ0
IKPCoWtJaJEhjkKtayj7k/xIme3LBDJzfJ6/k/x8o3qDLsra6H21rI7JoxnVRxKI
bqGIJ8eETdsPLil0xutAN0SiHKaRITPPm0RfK+R+ynhFofn3tTU=
=QRL7
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: Proposal: use Files.move instead of File.renameTo in FarmWarDeployer

Posted by Mark Thomas <ma...@apache.org>.
On 06/08/2019 21:05, Christopher Schultz wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> All,
> 
> Someone recently had a problem where the FarmWarDeployer wouldn't work
> on a secondary node because File.renameTo was failing -- likely due to
> the underlying Java/OS refusing to re-name a file across filesystems.
> 
> I propose that we switch to using Files.move which will either re-name
> or move depending upon what is necessary. It also throws an exception
> if it can't do its work, rather than failing and returning false.
> 
> Code patch below. I would also remove all of the
> "farmWarDeployer.renameFail" error message keys from the resource bundle
> s.

+1. Might be worth a wider review of where else File.renameTo() is used.

This is Java 7 so it can also be back-ported to 8.5.x.

Mark


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org