You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jena.apache.org by rv...@apache.org on 2014/06/10 12:37:12 UTC
svn commit: r1601601 - in /jena/trunk/jena-tdb/src:
main/java/com/hp/hpl/jena/tdb/ main/java/com/hp/hpl/jena/tdb/base/file/
main/java/com/hp/hpl/jena/tdb/sys/ test/java/com/hp/hpl/jena/tdb/base/file/
Author: rvesse
Date: Tue Jun 10 10:37:11 2014
New Revision: 1601601
URL: http://svn.apache.org/r1601601
Log:
Rework the TDB disk location locking a little and improve test casesto better detect whether they are valid on the platform they are running on (JENA-648)
Also adds a flag for this feature to SystemTDB which is currently off by default until the code and implementation stabilises
Added:
jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/ProcessUtils.java
Modified:
jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java
jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java
jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/SystemTDB.java
jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java
Modified: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java?rev=1601601&r1=1601600&r2=1601601&view=diff
==============================================================================
--- jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java (original)
+++ jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/StoreConnection.java Tue Jun 10 10:37:11 2014
@@ -30,6 +30,7 @@ import com.hp.hpl.jena.tdb.base.file.Loc
import com.hp.hpl.jena.tdb.base.file.LocationLock;
import com.hp.hpl.jena.tdb.setup.DatasetBuilderStd ;
import com.hp.hpl.jena.tdb.store.DatasetGraphTDB ;
+import com.hp.hpl.jena.tdb.sys.SystemTDB;
import com.hp.hpl.jena.tdb.transaction.* ;
/** A StoreConnection is the reference to the underlying storage.
@@ -205,7 +206,13 @@ public class StoreConnection
ChannelManager.release(sConn.transactionManager.getJournal().getFilename()) ;
// Release the lock
- location.getLock().release();
+ if (SystemTDB.DiskLocationMultiJvmUsagePrevention) {
+ if (location.getLock().isOwned()) {
+ location.getLock().release();
+ } else if (location.getLock().canLock()) {
+ SystemTDB.errlog.warn("Location " + location.getDirectoryPath() + " was not locked, if another JVM accessed this location simultaneously data corruption may have occurred");
+ }
+ }
}
/**
@@ -239,20 +246,23 @@ public class StoreConnection
{
sConn = new StoreConnection(dsg) ;
- // Obtain the lock ASAP
-// LocationLock lock = location.getLock();
-// if (lock.canLock()) {
-// if (!lock.canObtain())
-// throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is already locked by the process with PID " + lock.getOwner());
-//
-// lock.obtain();
-// // There's an interesting race condition here that two JVMs might write out the lock file one after another without
-// // colliding and causing an IO error in either. The best way to check for this is simply to check we now own the lock
-// // and if not error
-// if (!lock.isOwned()) {
-// throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is alread locked by the process with PID " + lock.getOwner());
-// }
-// }
+ if (SystemTDB.DiskLocationMultiJvmUsagePrevention)
+ {
+ // Obtain the lock ASAP
+ LocationLock lock = location.getLock();
+ if (lock.canLock()) {
+ if (!lock.canObtain())
+ throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is already locked by the process with PID " + lock.getOwner());
+
+ lock.obtain();
+ // There's an interesting race condition here that two JVMs might write out the lock file one after another without
+ // colliding and causing an IO error in either. The best way to check for this is simply to check we now own the lock
+ // and if not error
+ if (!lock.isOwned()) {
+ throw new TDBException("Can't open database at location " + location.getDirectoryPath() + " as it is alread locked by the process with PID " + lock.getOwner());
+ }
+ }
+ }
sConn.forceRecoverFromJournal() ;
// boolean actionTaken = JournalControl.recoverFromJournal(dsg.getConfig(), sConn.transactionManager.getJournal()) ;
Modified: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java?rev=1601601&r1=1601600&r2=1601601&view=diff
==============================================================================
--- jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java (original)
+++ jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/base/file/LocationLock.java Tue Jun 10 10:37:11 2014
@@ -1,69 +1,41 @@
+/*
+ * 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 com.hp.hpl.jena.tdb.base.file;
-import java.io.BufferedReader;
+
import java.io.BufferedWriter;
import java.io.File;
import java.io.FileWriter;
import java.io.IOException;
-import java.io.InputStreamReader;
-import java.lang.management.ManagementFactory;
-import java.util.ArrayList;
-import java.util.List;
-
import org.apache.jena.atlas.io.IO;
import com.hp.hpl.jena.tdb.TDBException;
-import com.hp.hpl.jena.tdb.sys.SystemTDB;
+import com.hp.hpl.jena.tdb.sys.ProcessUtils;
/**
* Represents a lock on a TDB location
*
- * @author rvesse
- *
*/
public class LocationLock {
private static final int NO_OWNER = 0;
private static final String LOCK_FILENAME = "tdb.lock";
- private static int myPid = -1;
-
- /**
- * Tries to get the PID of the current process
- *
- * @return PID of current process or zero if unable to determine PID
- */
- private static int getPid() {
- if (myPid != -1)
- return myPid;
-
- String runtimeBeanName = ManagementFactory.getRuntimeMXBean().getName();
- if (runtimeBeanName == null) {
- return useFallbackPid();
- }
-
- // Bean name will have format PID@hostname so we try to parse the PID
- // portion
- int index = runtimeBeanName.indexOf("@");
- if (index < 0)
- return useFallbackPid();
- try {
- // Parse and cache for future reuse
- String pidData = runtimeBeanName.substring(0, index);
- myPid = Integer.parseInt(pidData);
- return myPid;
- } catch (NumberFormatException e) {
- // Invalid PID
- return useFallbackPid();
- }
- }
-
- private static int useFallbackPid() {
- // In the case where we can't determine our PID then treat ourselves as
- // no owner and cache for future use
- myPid = NO_OWNER;
- return myPid;
- }
-
private Location location;
public LocationLock(Location location) {
@@ -111,7 +83,7 @@ public class LocationLock {
if (owner == NO_OWNER)
return false;
- return owner == getPid();
+ return owner == ProcessUtils.getPid(NO_OWNER);
}
/**
@@ -156,7 +128,7 @@ public class LocationLock {
return false;
int owner = this.getOwner();
- int pid = getPid();
+ int pid = ProcessUtils.getPid(NO_OWNER);
if (owner == NO_OWNER) {
// Can obtain provided we have a valid PID
@@ -168,7 +140,7 @@ public class LocationLock {
}
// Owned by another process, only obtainable if other process is dead
- if (!isAlive(owner))
+ if (!ProcessUtils.isAlive(owner))
return true;
// Otherwise not obtainable
@@ -187,7 +159,7 @@ public class LocationLock {
int owner = this.getOwner();
if (owner == NO_OWNER) {
// No owner currently so try to obtain the lock
- int pid = getPid();
+ int pid = ProcessUtils.getPid(NO_OWNER);
if (pid == NO_OWNER) {
// In the case where we cannot obtain our PID then we cannot
// obtain a lock
@@ -195,12 +167,12 @@ public class LocationLock {
}
takeLock(pid);
- } else if (owner == getPid()) {
+ } else if (owner == ProcessUtils.getPid(NO_OWNER)) {
// We already own the lock so nothing to do
} else {
// Someone other process potentially owns the lock on this location
// Check if the owner is alive
- if (isAlive(owner))
+ if (ProcessUtils.isAlive(owner))
throw new TDBException(
"The location "
+ location.getDirectoryPath()
@@ -209,7 +181,7 @@ public class LocationLock {
+ ". TDB databases do not permit concurrent usage across JVMs so in order to prevent corruption you cannot open this location from the JVM that does not own the lock for the dataset");
// Otherwise the previous owner is dead so we can take the lock
- takeLock(getPid());
+ takeLock(ProcessUtils.getPid(NO_OWNER));
}
}
@@ -241,7 +213,7 @@ public class LocationLock {
return;
// Some other process owns the lock so we can't release it
- if (owner != getPid())
+ if (owner != ProcessUtils.getPid(NO_OWNER))
throw new TDBException("Cannot release the lock on location " + location.getDirectoryPath()
+ " since this process does not own the lock");
@@ -300,59 +272,10 @@ public class LocationLock {
return;
if (!lockFile.isFile() || !lockFile.canWrite()) {
- // TODO What about read only file systems? Though I suspect TDB will
- // fail elsewhere in that case
-
// Unable to read lock owner because it isn't a file or we don't
// have read permission
throw new FileException(
"Unable to check TDB lock owner for this location since the expected lock file is not a file/not writable");
}
}
-
- private static boolean isAlive(int pid) {
- String pidStr = Integer.toString(pid);
- Process p;
- try {
- if (SystemTDB.isWindows) {
- // Use the Windows tasklist utility
- ProcessBuilder builder = new ProcessBuilder("tasklist", "/FI", "PID eq " + pidStr);
- builder.redirectErrorStream(true);
- p = builder.start();
- } else {
- // Use the ps utility
- ProcessBuilder builder = new ProcessBuilder("ps", "-p", pidStr);
- builder.redirectErrorStream(true);
- p = builder.start();
- }
-
- // Run and read data from the process
- BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream()));
-
- List<String> data = new ArrayList<String>();
- String line = null;
- while ((line = reader.readLine()) != null) {
- data.add(line);
- }
- reader.close();
-
- // Expect a line to contain the PID to indicate the process is
- // alive
- for (String lineData : data) {
- if (lineData.contains(pidStr))
- return true;
- }
-
- // Did not find any lines mentioning the PID so we can safely
- // assume that process is dead
- return false;
- } catch (IOException e) {
- // If any error running the process to check for the live process
- // then our check failed and for safety we assume the process is
- // alive
-
- // TODO Issue a warning here
- return true;
- }
- }
}
Added: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/ProcessUtils.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/ProcessUtils.java?rev=1601601&view=auto
==============================================================================
--- jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/ProcessUtils.java (added)
+++ jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/ProcessUtils.java Tue Jun 10 10:37:11 2014
@@ -0,0 +1,159 @@
+/*
+ * 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 com.hp.hpl.jena.tdb.sys;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.lang.management.ManagementFactory;
+import java.util.ArrayList;
+import java.util.List;
+
+public class ProcessUtils {
+
+ private static int myPid = -1;
+
+ /**
+ * Tries to get the PID of the current process caching it for future calls
+ * since it won't change throughout the life of the process
+ *
+ * @param fallback
+ * Fallback PID to return if unable to determine the PID i.e. an
+ * error code to return
+ *
+ * @return PID of current process or provided {@code fallback} if unable to
+ * determine PID
+ */
+ public static int getPid(int fallback) {
+ if (myPid != -1)
+ return myPid;
+
+ String runtimeBeanName = ManagementFactory.getRuntimeMXBean().getName();
+ if (runtimeBeanName == null) {
+ return useFallbackPid(fallback);
+ }
+
+ // Bean name will have format PID@hostname so we try to parse the PID
+ // portion
+ int index = runtimeBeanName.indexOf("@");
+ if (index < 0)
+ return useFallbackPid(fallback);
+ try {
+ // Parse and cache for future reuse
+ String pidData = runtimeBeanName.substring(0, index);
+ myPid = Integer.parseInt(pidData);
+ return myPid;
+ } catch (NumberFormatException e) {
+ // Invalid PID
+ return useFallbackPid(fallback);
+ }
+ }
+
+ private static int useFallbackPid(int fallback) {
+ // In the case where we can't determine our PID then treat ourselves as
+ // no owner and cache for future use
+ myPid = fallback;
+ return myPid;
+ }
+
+ /**
+ * Determines whether a given PID is alive
+ *
+ * @param pid
+ * PID
+ * @return True if the given PID is alive or unknown, false if it is dead
+ */
+ public static boolean isAlive(int pid) {
+ String pidStr = Integer.toString(pid);
+ try {
+ List<String> data = getProcessInfo(pidStr);
+
+ // Expect a line to contain the PID to indicate the process is
+ // alive
+ for (String lineData : data) {
+ if (lineData.contains(pidStr))
+ return true;
+ }
+
+ // Did not find any lines mentioning the PID so we can safely
+ // assume that process is dead
+ return false;
+ } catch (IOException e) {
+ // If any error running the process to check for the live process
+ // then our check failed and for safety we assume the process is
+ // alive
+
+ SystemTDB.errlog
+ .warn("Your platform does not support checking process liveness so TDB disk locations cannot be reliably locked to prevent possible corruption due to unsafe multi-JVM usage");
+ return true;
+ }
+ }
+
+ /**
+ * Gets whether the platform we are running on will cause us to treat
+ * negative (i.e. invalid) PIDs as alive because of the format in which the
+ * command line process monitor application for the system returns errors
+ * for invalid PIDs
+ *
+ * @return True if a negative PID is treated as alive on this platform or we
+ * cannot determine liveness for PIDs on this platform, false
+ * otherwise
+ */
+ public static boolean negativePidsTreatedAsAlive() {
+ return isAlive(-1);
+ }
+
+ /**
+ * Gets process information based on the given PID string
+ *
+ * @param pidStr
+ * PID string
+ * @return Output of running the OSes appropriate command line task info
+ * application
+ * @throws IOException
+ * Thrown if there is a problem running the command line
+ * application or reading the data returned
+ */
+ private static List<String> getProcessInfo(String pidStr) throws IOException {
+ Process p;
+ if (SystemTDB.isWindows) {
+ // Use the Windows tasklist utility
+ ProcessBuilder builder = new ProcessBuilder("tasklist", "/FI", "PID eq " + pidStr);
+ builder.redirectErrorStream(true);
+ p = builder.start();
+ } else {
+ // Use the ps utility
+ ProcessBuilder builder = new ProcessBuilder("ps", "-p", pidStr);
+ builder.redirectErrorStream(true);
+ p = builder.start();
+ }
+
+ // Run and read data from the process
+ BufferedReader reader = new BufferedReader(new InputStreamReader(p.getInputStream()));
+
+ List<String> data = new ArrayList<String>();
+ String line = null;
+ while ((line = reader.readLine()) != null) {
+ data.add(line);
+ }
+ reader.close();
+
+ return data;
+ }
+}
Modified: jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/SystemTDB.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/SystemTDB.java?rev=1601601&r1=1601600&r2=1601601&view=diff
==============================================================================
--- jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/SystemTDB.java (original)
+++ jena/trunk/jena-tdb/src/main/java/com/hp/hpl/jena/tdb/sys/SystemTDB.java Tue Jun 10 10:37:11 2014
@@ -206,6 +206,18 @@ public class SystemTDB
public static final byte FillByte = (byte)0xFF ;
public static boolean Checking = false ; // This isn't used enough!
+
+ /**
+ * New feature introduced by JENA-648 to help prevent one common cause of JVM corruption.
+ * <p>
+ * When enabled lock files are written to disk locations with the current owner process PID,
+ * other processes will refuse to access that location while another live process owns the PID.
+ * </p>
+ * <p>
+ * Currently off by default as the new code stabilises and is debugged
+ * </p>
+ */
+ public static boolean DiskLocationMultiJvmUsagePrevention = false;
// BDB related.
//public static final int BDB_cacheSizePercent = intValue("BDB_cacheSizePercent", 75) ;
Modified: jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java
URL: http://svn.apache.org/viewvc/jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java?rev=1601601&r1=1601600&r2=1601601&view=diff
==============================================================================
--- jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java (original)
+++ jena/trunk/jena-tdb/src/test/java/com/hp/hpl/jena/tdb/base/file/TestLocationLock.java Tue Jun 10 10:37:11 2014
@@ -4,22 +4,41 @@ import java.io.BufferedWriter;
import java.io.FileWriter;
import java.io.IOException;
+import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Assume;
-import org.junit.Ignore;
+import org.junit.BeforeClass;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import com.hp.hpl.jena.tdb.StoreConnection;
import com.hp.hpl.jena.tdb.TDBException;
+import com.hp.hpl.jena.tdb.sys.ProcessUtils;
import com.hp.hpl.jena.tdb.sys.SystemTDB;
-@Ignore
+/**
+ * Tests for {@link LocationLock}
+ * @author rvesse
+ *
+ */
public class TestLocationLock {
+ private static boolean negativePidsTreatedAsAlive = false;
+
@Rule
public TemporaryFolder tempDir = new TemporaryFolder();
+
+ @BeforeClass
+ public static void setup() {
+ SystemTDB.DiskLocationMultiJvmUsagePrevention = true;
+ negativePidsTreatedAsAlive = ProcessUtils.negativePidsTreatedAsAlive();
+ }
+
+ @AfterClass
+ public static void teardown() {
+ SystemTDB.DiskLocationMultiJvmUsagePrevention = false;
+ }
@Test
public void location_lock_mem() {
@@ -52,9 +71,8 @@ public class TestLocationLock {
}
@Test
- @Ignore
public void location_lock_dir_02() throws IOException {
- Assume.assumeFalse(SystemTDB.isWindows);
+ Assume.assumeTrue(negativePidsTreatedAsAlive);
Location dir = new Location(tempDir.getRoot().getAbsolutePath());
LocationLock lock = dir.getLock();
@@ -71,7 +89,7 @@ public class TestLocationLock {
writer.close();
Assert.assertTrue(lock.isLocked());
Assert.assertFalse(lock.isOwned());
- Assert.assertFalse(lock.canObtain()); // Returns true on Jenkins
+ Assert.assertFalse(lock.canObtain());
}
@Test
@@ -97,9 +115,8 @@ public class TestLocationLock {
}
@Test(expected = TDBException.class)
- @Ignore
public void location_lock_dir_error_01() throws IOException {
- Assume.assumeFalse(SystemTDB.isWindows);
+ Assume.assumeTrue(negativePidsTreatedAsAlive);
Location dir = new Location(tempDir.getRoot().getAbsolutePath());
LocationLock lock = dir.getLock();
@@ -118,14 +135,13 @@ public class TestLocationLock {
Assert.assertFalse(lock.isOwned());
// Attempting to obtain the lock should now error
- Assert.assertFalse(lock.canObtain()); // Returns true on Jenkins
+ Assert.assertFalse(lock.canObtain());
lock.obtain();
}
@Test(expected = TDBException.class)
- @Ignore
public void location_lock_dir_error_02() throws IOException {
- Assume.assumeFalse(SystemTDB.isWindows);
+ Assume.assumeTrue(negativePidsTreatedAsAlive);
Location dir = new Location(tempDir.getRoot().getAbsolutePath());
LocationLock lock = dir.getLock();
@@ -149,9 +165,8 @@ public class TestLocationLock {
}
@Test(expected = TDBException.class)
- @Ignore
public void location_lock_dir_error_03() throws IOException {
- Assume.assumeFalse(SystemTDB.isWindows);
+ Assume.assumeTrue(negativePidsTreatedAsAlive);
Location dir = new Location(tempDir.getRoot().getAbsolutePath());
LocationLock lock = dir.getLock();