You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by re...@apache.org on 2015/10/30 07:37:28 UTC
svn commit: r1711405 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java MissingLastRevSeeker.java
mongo/MongoMissingLastRevSeeker.java
Author: reschke
Date: Fri Oct 30 06:37:28 2015
New Revision: 1711405
URL: http://svn.apache.org/viewvc?rev=1711405&view=rev
Log:
OAK-3439: MissingLastRevSeeker potential race condition acquiring the lock - use new notEquals condition in UpdateOp, remove Mongo specific code
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ClusterNodeInfo.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingLastRevSeeker.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoMissingLastRevSeeker.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ClusterNodeInfo.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ClusterNodeInfo.java?rev=1711405&r1=1711404&r2=1711405&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ClusterNodeInfo.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/ClusterNodeInfo.java Fri Oct 30 06:37:28 2015
@@ -656,7 +656,7 @@ public class ClusterNodeInfo {
now = getCurrentTime();
leaseEndTime = now + leaseTime;
}
- UpdateOp update = new UpdateOp("" + id, true);
+ UpdateOp update = new UpdateOp("" + id, false);
update.set(LEASE_END_KEY, leaseEndTime);
update.set(STATE, ClusterNodeState.ACTIVE.name());
ClusterNodeInfoDocument doc = null;
@@ -681,7 +681,7 @@ public class ClusterNodeInfo {
// this is only for startup - then we 'just' overwrite
// the lease - or create it - and don't care a lot about what the
// status of the lease was
- doc = store.createOrUpdate(Collection.CLUSTER_NODES, update);
+ doc = store.findAndUpdate(Collection.CLUSTER_NODES, update);
}
if (doc==null) { // should not occur when leaseCheckDisabled
// OAK-3398 : someone else either started recovering or is already through with that.
@@ -745,7 +745,7 @@ public class ClusterNodeInfo {
UpdateOp update = new UpdateOp("" + id, true);
update.set(LEASE_END_KEY, null);
update.set(STATE, null);
- update.set(REV_RECOVERY_LOCK, null);
+ update.set(REV_RECOVERY_LOCK, RecoverLockState.NONE.name());
store.createOrUpdate(Collection.CLUSTER_NODES, update);
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingLastRevSeeker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingLastRevSeeker.java?rev=1711405&r1=1711404&r2=1711405&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingLastRevSeeker.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/MissingLastRevSeeker.java Fri Oct 30 06:37:28 2015
@@ -86,18 +86,25 @@ public class MissingLastRevSeeker {
});
}
+ /**
+ * Acquire a recovery lock for the given cluster node info document
+ *
+ * @param clusterId
+ * id of the cluster that is going to be recovered
+ * @param recoveredBy
+ * id of cluster doing the recovery ({@code 0} when unknown)
+ * @return whether the lock has been acquired
+ */
public boolean acquireRecoveryLock(int clusterId, int recoveredBy) {
try {
- // This approach has a race condition where two different cluster
- // nodes
- // can acquire the lock simultaneously.
- UpdateOp update = new UpdateOp(Integer.toString(clusterId), true);
+ UpdateOp update = new UpdateOp(Integer.toString(clusterId), false);
+ update.notEquals(ClusterNodeInfo.REV_RECOVERY_LOCK, RecoverLockState.ACQUIRED.name());
update.set(ClusterNodeInfo.REV_RECOVERY_LOCK, RecoverLockState.ACQUIRED.name());
if (recoveredBy != 0) {
update.set(ClusterNodeInfo.REV_RECOVERY_BY, recoveredBy);
}
- store.createOrUpdate(Collection.CLUSTER_NODES, update);
- return true;
+ ClusterNodeInfoDocument old = store.findAndUpdate(Collection.CLUSTER_NODES, update);
+ return old != null;
} catch (RuntimeException ex) {
LOG.error("Failed to acquire the recovery lock for clusterNodeId " + clusterId, ex);
throw (ex);
@@ -106,11 +113,14 @@ public class MissingLastRevSeeker {
public void releaseRecoveryLock(int clusterId) {
try {
- UpdateOp update = new UpdateOp(Integer.toString(clusterId), true);
- update.set(ClusterNodeInfo.REV_RECOVERY_LOCK, null);
+ UpdateOp update = new UpdateOp(Integer.toString(clusterId), false);
+ update.set(ClusterNodeInfo.REV_RECOVERY_LOCK, RecoverLockState.NONE.name());
update.set(ClusterNodeInfo.REV_RECOVERY_BY, null);
update.set(ClusterNodeInfo.STATE, null);
- store.createOrUpdate(Collection.CLUSTER_NODES, update);
+ ClusterNodeInfoDocument old = store.findAndUpdate(Collection.CLUSTER_NODES, update);
+ if (old == null) {
+ throw new RuntimeException("ClusterNodeInfo document for " + clusterId + " missing.");
+ }
} catch (RuntimeException ex) {
LOG.error("Failed to release the recovery lock for clusterNodeId " + clusterId, ex);
throw (ex);
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoMissingLastRevSeeker.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoMissingLastRevSeeker.java?rev=1711405&r1=1711404&r2=1711405&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoMissingLastRevSeeker.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/mongo/MongoMissingLastRevSeeker.java Fri Oct 30 06:37:28 2015
@@ -33,7 +33,6 @@ import com.mongodb.ReadPreference;
import org.apache.jackrabbit.oak.plugins.document.ClusterNodeInfo;
import org.apache.jackrabbit.oak.plugins.document.Collection;
-import org.apache.jackrabbit.oak.plugins.document.Document;
import org.apache.jackrabbit.oak.plugins.document.MissingLastRevSeeker;
import org.apache.jackrabbit.oak.plugins.document.NodeDocument;
import org.apache.jackrabbit.oak.plugins.document.util.CloseableIterable;
@@ -73,33 +72,6 @@ public class MongoMissingLastRevSeeker e
}
@Override
- public boolean acquireRecoveryLock(int clusterId, int recoveredBy) {
- QueryBuilder query =
- start().and(
- start(Document.ID).is(Integer.toString(clusterId)).get(),
- start(ClusterNodeInfo.REV_RECOVERY_LOCK).notEquals(RecoverLockState.ACQUIRED.name()).get()
- );
-
- DBObject returnFields = new BasicDBObject();
- returnFields.put("_id", 1);
-
- BasicDBObject setUpdates = new BasicDBObject();
- setUpdates.append(ClusterNodeInfo.REV_RECOVERY_LOCK, RecoverLockState.ACQUIRED.name());
- if (recoveredBy != 0) {
- setUpdates.append(ClusterNodeInfo.REV_RECOVERY_BY, recoveredBy);
- }
-
- BasicDBObject update = new BasicDBObject();
- update.append("$set", setUpdates);
-
- DBObject oldNode = getClusterNodeCollection().findAndModify(query.get(), returnFields,
- null /*sort*/, false /*remove*/, update, false /*returnNew*/,
- false /*upsert*/);
-
- return oldNode != null;
- }
-
- @Override
public boolean isRecoveryNeeded(long currentTime) {
QueryBuilder query =
start(ClusterNodeInfo.STATE).is(ClusterNodeInfo.ClusterNodeState.ACTIVE.name())