Commit 66bd0584 authored by Rakesh Radhakrishnan's avatar Rakesh Radhakrishnan
Browse files

ZOOKEEPER-1835. dynamic configuration file renaming fails on Windows(Bruno...

ZOOKEEPER-1835. dynamic configuration file renaming fails on Windows(Bruno Freudensprung via rakeshr)

git-svn-id: https://svn.apache.org/repos/asf/zookeeper/trunk@1607774 13f79535-47bb-0310-9956-ffa450edef68
parent daab8e16
......@@ -678,6 +678,9 @@ BUGFIXES:
ZOOKEEPER-1939. ReconfigRecoveryTest.testNextConfigUnreachable is
failing (Rakesh R via phunt)
ZOOKEEPER-1835. dynamic configuration file renaming fails on Windows
(Bruno Freudensprung via rakeshr)
IMPROVEMENTS:
ZOOKEEPER-1170. Fix compiler (eclipse) warnings: unused imports,
......
......@@ -68,7 +68,7 @@ public class AtomicFileOutputStream extends FilterOutputStream {
boolean triedToClose = false, success = false;
try {
flush();
((FileOutputStream) out).getChannel().force(true);
((FileOutputStream) out).getFD().sync();
triedToClose = true;
super.close();
......
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.zookeeper.common;
import java.io.BufferedWriter;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
/*
* Used to perform an atomic write into a file.
* If there is a failure in the middle of the writing operation,
* the original file (if it exists) is left intact.
* Based on the org.apache.zookeeper.server.quorum.QuorumPeer.writeLongToFile(...) idiom
* using the HDFS AtomicFileOutputStream class.
*/
public class AtomicFileWritingIdiom {
public static interface OutputStreamStatement {
public void write(OutputStream os) throws IOException;
}
public static interface WriterStatement {
public void write(Writer os) throws IOException;
}
public AtomicFileWritingIdiom(File targetFile, OutputStreamStatement osStmt) throws IOException {
this(targetFile, osStmt, null);
}
public AtomicFileWritingIdiom(File targetFile, WriterStatement wStmt) throws IOException {
this(targetFile, null, wStmt);
}
private AtomicFileWritingIdiom(File targetFile, OutputStreamStatement osStmt, WriterStatement wStmt) throws IOException {
AtomicFileOutputStream out = null;
boolean error = true;
try {
out = new AtomicFileOutputStream(targetFile);
if (wStmt == null) {
// execute output stream operation
osStmt.write(out);
} else {
BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(out));
// execute writer operation and flush
wStmt.write(bw);
bw.flush();
}
out.flush();
// everything went ok
error = false;
} finally {
// nothing interesting to do if out == null
if (out != null) {
if (error) {
// worst case here the tmp file/resources(fd) are not cleaned up
// and the caller will be notified (IOException)
out.abort();
} else {
// if the close operation (rename) fails we'll get notified.
// worst case the tmp file may still exist
IOUtils.closeStream(out);
}
}
}
}
}
......@@ -28,6 +28,7 @@ import java.io.StringWriter;
import java.io.OutputStreamWriter;
import java.io.StringReader;
import java.io.StringWriter;
import java.io.Writer;
import java.net.DatagramPacket;
import java.net.DatagramSocket;
import java.net.InetSocketAddress;
......@@ -46,7 +47,8 @@ import java.util.Set;
import org.apache.zookeeper.KeeperException.NoNodeException;
import org.apache.zookeeper.ZooDefs;
import org.apache.zookeeper.common.AtomicFileOutputStream;
import org.apache.zookeeper.common.AtomicFileWritingIdiom;
import org.apache.zookeeper.common.AtomicFileWritingIdiom.WriterStatement;
import org.apache.zookeeper.common.HostNameUtils;
import org.apache.zookeeper.jmx.MBeanRegistry;
import org.apache.zookeeper.jmx.ZKMBeanInfo;
......@@ -1488,30 +1490,14 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
* @param value the long value to write to the named file
* @throws IOException if the file cannot be written atomically
*/
private void writeLongToFile(String name, long value) throws IOException {
private void writeLongToFile(String name, final long value) throws IOException {
File file = new File(logFactory.getSnapDir(), name);
AtomicFileOutputStream out = new AtomicFileOutputStream(file);
BufferedWriter bw = new BufferedWriter(new OutputStreamWriter(out));
boolean aborted = false;
try {
bw.write(Long.toString(value));
bw.flush();
out.flush();
} catch (IOException e) {
LOG.error("Failed to write new file " + file, e);
// worst case here the tmp file/resources(fd) are not cleaned up
// and the caller will be notified (IOException)
aborted = true;
out.abort();
throw e;
} finally {
if (!aborted) {
// if the close operation (rename) fails we'll get notified.
// worst case the tmp file may still exist
out.close();
new AtomicFileWritingIdiom(file, new WriterStatement() {
@Override
public void write(Writer bw) throws IOException {
bw.write(Long.toString(value));
}
}
});
}
public long getCurrentEpoch() throws IOException {
......
......@@ -26,6 +26,8 @@ import java.io.FileOutputStream;
import java.io.FileReader;
import java.io.FileWriter;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.Collections;
......@@ -36,8 +38,9 @@ import java.util.Map.Entry;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.slf4j.MDC;
import org.apache.zookeeper.common.AtomicFileWritingIdiom;
import org.apache.zookeeper.common.AtomicFileWritingIdiom.OutputStreamStatement;
import org.apache.zookeeper.common.AtomicFileWritingIdiom.WriterStatement;
import org.apache.zookeeper.server.ZooKeeperServer;
import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
......@@ -299,74 +302,55 @@ public class QuorumPeerConfig {
* @param qv
*/
public static void writeDynamicConfig(String dynamicConfigFilename, String configFileStr,
boolean configBackwardCompatibilityMode, QuorumVerifier qv) throws IOException {
FileOutputStream outConfig = null;
try {
byte b[] = qv.toString().getBytes();
if (configBackwardCompatibilityMode) {
dynamicConfigFilename = configFileStr + ".dynamic";
}
String tmpFilename = dynamicConfigFilename + ".tmp";
outConfig = new FileOutputStream(tmpFilename);
outConfig.write(b);
outConfig.close();
File curFile = new File(dynamicConfigFilename);
File tmpFile = new File(tmpFilename);
if (!tmpFile.renameTo(curFile)) {
throw new IOException("renaming " + tmpFile.toString() + " to " + curFile.toString() + " failed!");
}
} finally{
if (outConfig!=null) {
outConfig.close();
}
}
// the following is for users who run without a dynamic config file (old config file)
// if the configuration changes (reconfiguration executes), we create a dynamic config
// file, remove all the dynamic definitions from the config file and add a pointer
// to the config file. The dynamic config file's name will be the same as the config file's
// with ".dynamic" appended to it
boolean configBackwardCompatibilityMode, final QuorumVerifier qv) throws IOException {
if (configBackwardCompatibilityMode) {
dynamicConfigFilename = configFileStr + ".dynamic";
}
final String actualDynamicConfigFilename = dynamicConfigFilename;
new AtomicFileWritingIdiom(new File(actualDynamicConfigFilename), new OutputStreamStatement() {
@Override
public void write(OutputStream outConfig) throws IOException {
byte b[] = qv.toString().getBytes();
outConfig.write(b);
}
});
// the following is for users who run without a dynamic config file (old config file)
// if the configuration changes (reconfiguration executes), we create a dynamic config
// file, remove all the dynamic definitions from the config file and add a pointer
// to the config file. The dynamic config file's name will be the same as the config file's
// with ".dynamic" appended to it
if (configBackwardCompatibilityMode) {
BufferedWriter out = null;
try {
File configFile = (new VerifyingFileFactory.Builder(LOG)
.warnForRelativePath()
.failForNonExistingPath()
.build()).create(configFileStr);
Properties cfg = new Properties();
FileInputStream in = new FileInputStream(configFile);
try {
cfg.load(in);
} finally {
in.close();
}
String tmpFilename = configFileStr + ".tmp";
FileWriter fstream = new FileWriter(tmpFilename);
out = new BufferedWriter(fstream);
File configFile = (new VerifyingFileFactory.Builder(LOG)
.warnForRelativePath()
.failForNonExistingPath()
.build()).create(configFileStr);
for (Entry<Object, Object> entry : cfg.entrySet()) {
String key = entry.getKey().toString().trim();
String value = entry.getValue().toString().trim();
if (!key.startsWith("server.") && !key.startsWith("group")
&& !key.startsWith("weight") && !key.equals("clientPort") && !key.equals("clientPortAddress")){
out.write(key.concat("=").concat(value).concat("\n"));
}
}
out.write("dynamicConfigFile=".concat(dynamicConfigFilename).concat("\n"));
out.close();
File tmpFile = new File(tmpFilename);
if (!tmpFile.renameTo(configFile)) {
throw new IOException("renaming " + tmpFile.toString() + " to " + configFile.toString() + " failed!");
}
} finally{
if (out!=null) {
out.close();
}
final Properties cfg = new Properties();
FileInputStream in = new FileInputStream(configFile);
try {
cfg.load(in);
} finally {
in.close();
}
}
}
new AtomicFileWritingIdiom(new File(configFileStr), new WriterStatement() {
@Override
public void write(Writer out) throws IOException {
for (Entry<Object, Object> entry : cfg.entrySet()) {
String key = entry.getKey().toString().trim();
String value = entry.getValue().toString().trim();
if (!key.startsWith("server.") && !key.startsWith("group")
&& !key.startsWith("weight") && !key.equals("clientPort") && !key.equals("clientPortAddress")){
out.write(key.concat("=").concat(value).concat("\n"));
}
}
out.write("dynamicConfigFile=".concat(actualDynamicConfigFilename).concat("\n"));
}
});
}
}
public static void deleteFile(String filename){
File f = new File(filename);
if (f.exists()) {
......
/**
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.apache.zookeeper.common;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.Writer;
import org.apache.zookeeper.ZKTestCase;
import org.apache.zookeeper.common.AtomicFileWritingIdiom.OutputStreamStatement;
import org.apache.zookeeper.common.AtomicFileWritingIdiom.WriterStatement;
import org.junit.BeforeClass;
import org.junit.Test;
public class AtomicFileWritingIdiomTest extends ZKTestCase {
private static File tmpdir;
@BeforeClass
public static void createTmpDir() {
tmpdir = new File("build/test/tmp");
tmpdir.mkdirs();
}
@Test
public void testOutputStreamSuccess() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
@Override
public void write(OutputStream os) throws IOException {
os.write("after".getBytes("ASCII"));
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
}
});
assertFalse("tmp file should have been deleted", tmp.exists());
// content changed
assertEquals("after", getContent(target));
target.delete();
}
@Test
public void testWriterSuccess() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
new AtomicFileWritingIdiom(target, new WriterStatement() {
@Override
public void write(Writer os) throws IOException {
os.write("after");
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
}
});
assertFalse("tmp file should have been deleted", tmp.exists());
// content changed
assertEquals("after", getContent(target));
target.delete();
}
@Test
public void testOutputStreamFailure() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
boolean exception = false;
try {
new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
@Override
public void write(OutputStream os) throws IOException {
os.write("after".getBytes("ASCII"));
os.flush();
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
throw new RuntimeException();
}
});
} catch (RuntimeException ex) {
exception = true;
}
assertFalse("tmp file should have been deleted", tmp.exists());
assertTrue("should have raised an exception", exception);
// content preserved
assertEquals("before", getContent(target));
target.delete();
}
@Test
public void testWriterFailure() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
boolean exception = false;
try {
new AtomicFileWritingIdiom(target, new WriterStatement() {
@Override
public void write(Writer os) throws IOException {
os.write("after");
os.flush();
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
throw new RuntimeException();
}
});
} catch (RuntimeException ex) {
exception = true;
}
assertFalse("tmp file should have been deleted", tmp.exists());
assertTrue("should have raised an exception", exception);
// content preserved
assertEquals("before", getContent(target));
target.delete();
}
@Test
public void testOutputStreamFailureIOException() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
boolean exception = false;
try {
new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
@Override
public void write(OutputStream os) throws IOException {
os.write("after".getBytes("ASCII"));
os.flush();
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
throw new IOException();
}
});
} catch (IOException ex) {
exception = true;
}
assertFalse("tmp file should have been deleted", tmp.exists());
assertTrue("should have raised an exception", exception);
// content preserved
assertEquals("before", getContent(target));
target.delete();
}
@Test
public void testWriterFailureIOException() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
boolean exception = false;
try {
new AtomicFileWritingIdiom(target, new WriterStatement() {
@Override
public void write(Writer os) throws IOException {
os.write("after");
os.flush();
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
throw new IOException();
}
});
} catch (IOException ex) {
exception = true;
}
assertFalse("tmp file should have been deleted", tmp.exists());
assertTrue("should have raised an exception", exception);
// content preserved
assertEquals("before", getContent(target));
target.delete();
}
@Test
public void testOutputStreamFailureError() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
boolean exception = false;
try {
new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
@Override
public void write(OutputStream os) throws IOException {
os.write("after".getBytes("ASCII"));
os.flush();
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
throw new Error();
}
});
} catch (Error ex) {
exception = true;
}
assertFalse("tmp file should have been deleted", tmp.exists());
assertTrue("should have raised an exception", exception);
// content preserved
assertEquals("before", getContent(target));
target.delete();
}
@Test
public void testWriterFailureError() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
createFile(target, "before");
assertEquals("before", getContent(target));
boolean exception = false;
try {
new AtomicFileWritingIdiom(target, new WriterStatement() {
@Override
public void write(Writer os) throws IOException {
os.write("after");
os.flush();
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
throw new Error();
}
});
} catch (Error ex) {
exception = true;
}
assertFalse("tmp file should have been deleted", tmp.exists());
assertTrue("should have raised an exception", exception);
// content preserved
assertEquals("before", getContent(target));
target.delete();
}
// ************** target file does not exist
@Test
public void testOutputStreamSuccessNE() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");
target.delete();
assertFalse("file should not exist", target.exists());
new AtomicFileWritingIdiom(target, new OutputStreamStatement() {
@Override
public void write(OutputStream os) throws IOException {
os.write("after".getBytes("ASCII"));
assertTrue("implementation of AtomicFileOutputStream has changed, update the test", tmp.exists());
}
});
// content changed
assertEquals("after", getContent(target));
target.delete();
}
@Test
public void testWriterSuccessNE() throws IOException {
File target = new File(tmpdir, "target.txt");
final File tmp = new File(tmpdir, "target.txt.tmp");