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 st...@apache.org on 2015/08/17 09:47:15 UTC
svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDocumentStoreWrapper.java
Author: stefanegli
Date: Mon Aug 17 07:47:15 2015
New Revision: 1696202
URL: http://svn.apache.org/r1696202
Log:
OAK-2739 : lease check introduced : by default there's now a check active which assures the local lease is valid upon every action done towards the DocumentStore
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LeaseCheckDocumentStoreWrapper.java (with props)
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/DocumentMK.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.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=1696202&r1=1696201&r2=1696202&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 Mon Aug 17 07:47:15 2015
@@ -177,7 +177,7 @@ public class ClusterNodeInfo {
/**
* The time (in milliseconds UTC) where the lease of this instance ends.
*/
- private long leaseEndTime;
+ private volatile long leaseEndTime;
/**
* The read/write mode.
@@ -188,8 +188,20 @@ public class ClusterNodeInfo {
* The state of the cluter node.
*/
private ClusterNodeState state;
+
+ /**
+ * Whether or not the OAK-2739/leaseCheck failed and thus a System.exit was already triggered
+ * (is used to avoid calling System.exit a hundred times when it then happens)
+ */
+ private volatile boolean systemExitTriggered;
/**
+ * Tracks the fact whether the lease has *ever* been renewed by this instance
+ * or has just be read from the document store at initialization time.
+ */
+ private boolean renewed;
+
+ /**
* The revLock value of the cluster;
*/
private RecoverLockState revRecoveryLock;
@@ -212,6 +224,7 @@ public class ClusterNodeInfo {
} else {
this.leaseEndTime = leaseEnd;
}
+ this.renewed = false; // will be updated once we renew it the first time
this.store = store;
this.machineId = machineId;
this.instanceId = instanceId;
@@ -356,6 +369,49 @@ public class ClusterNodeInfo {
RecoverLockState.NONE, prevLeaseEnd, newEntry);
}
+ public void performLeaseCheck() {
+ if (!renewed) {
+ // the 'renewed' flag indicates if this instance *ever* renewed the lease after startup
+ // until that is not set, we cannot do the lease check (otherwise startup wouldn't work)
+ return;
+ }
+ final long now = getCurrentTime();
+ if (now < leaseEndTime) {
+ // then all is good
+ return;
+ }
+
+ // OAK-2739 : when the lease is not current, we must stop
+ // the instance immediately to avoid any cluster inconsistency
+ final String errorMsg = "performLeaseCheck: this instance failed to update the lease in time "
+ + "(leaseEndTime: "+leaseEndTime+", now: "+now+", leaseTime: "+leaseTime+") "
+ + "and is thus no longer eligible for taking part in the cluster. Shutting down NOW!";
+ LOG.error(errorMsg);
+
+ // now here comes the thing: we should a) call System.exit in a separate thread
+ // to avoid any deadlock when calling from eg within the shutdown hook
+ // AND b) we should not call system.exit hundred times.
+ // so for b) we use 'systemExitTriggered' to avoid calling it over and over
+ // BUT it doesn't have to be 100% ensured that system.exit is called only once.
+ // it is fine if it gets called once, twice - but just not hundred times.
+ // which is a long way of saying: volatile is fine here - and the 'if' too
+ if (!systemExitTriggered) {
+ systemExitTriggered = true;
+ final Runnable r = new Runnable() {
+
+ @Override
+ public void run() {
+ System.exit(-1);
+ }
+
+ };
+ final Thread th = new Thread(r, "FailedLeaseCheckShutdown-Thread");
+ th.setDaemon(true);
+ th.start();
+ }
+ throw new AssertionError(errorMsg);
+ }
+
/**
* Renew the cluster id lease. This method needs to be called once in a while,
* to ensure the same cluster id is not re-used by a different instance.
@@ -379,6 +435,7 @@ public class ClusterNodeInfo {
readWriteMode = mode;
store.setReadWriteMode(mode);
}
+ renewed = true;
return true;
}
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java?rev=1696202&r1=1696201&r2=1696202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentMK.java Mon Aug 17 07:47:15 2015
@@ -474,6 +474,7 @@ public class DocumentMK {
private int asyncDelay = 1000;
private boolean timing;
private boolean logging;
+ private boolean leaseCheck;
private Weigher<CacheValue, CacheValue> weigher = new EmpiricalWeigher();
private long memoryCacheSize = DEFAULT_MEMORY_CACHE_SIZE;
private int nodeCachePercentage = DEFAULT_NODE_CACHE_PERCENTAGE;
@@ -601,6 +602,15 @@ public class DocumentMK {
public boolean getLogging() {
return logging;
}
+
+ public Builder setLeaseCheck(boolean leaseCheck) {
+ this.leaseCheck = leaseCheck;
+ return this;
+ }
+
+ public boolean getLeaseCheck() {
+ return leaseCheck;
+ }
/**
* Set the document store to use. By default an in-memory store is used.
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1696202&r1=1696201&r2=1696202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Mon Aug 17 07:47:15 2015
@@ -96,6 +96,7 @@ import org.apache.jackrabbit.oak.cache.C
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.json.BlobSerializer;
import org.apache.jackrabbit.oak.plugins.document.Branch.BranchCommit;
+import org.apache.jackrabbit.oak.plugins.document.util.LeaseCheckDocumentStoreWrapper;
import org.apache.jackrabbit.oak.plugins.document.util.LoggingDocumentStoreWrapper;
import org.apache.jackrabbit.oak.plugins.document.util.StringValue;
import org.apache.jackrabbit.oak.plugins.document.util.TimingDocumentStoreWrapper;
@@ -411,14 +412,13 @@ public final class DocumentNodeStore
if (builder.getLogging()) {
s = new LoggingDocumentStoreWrapper(s);
}
- this.store = s;
this.changes = Collection.JOURNAL.newDocument(s);
this.executor = builder.getExecutor();
this.clock = builder.getClock();
int cid = builder.getClusterId();
cid = Integer.getInteger("oak.documentMK.clusterId", cid);
if (cid == 0) {
- clusterNodeInfo = ClusterNodeInfo.getInstance(store);
+ clusterNodeInfo = ClusterNodeInfo.getInstance(s);
// TODO we should ensure revisions generated from now on
// are never "older" than revisions already in the repository for
// this cluster id
@@ -426,6 +426,10 @@ public final class DocumentNodeStore
} else {
clusterNodeInfo = null;
}
+ if (builder.getLeaseCheck()) {
+ s = new LeaseCheckDocumentStoreWrapper(s, clusterNodeInfo);
+ }
+ this.store = s;
this.clusterId = cid;
this.revisionComparator = new Revision.RevisionComparator(clusterId);
this.branches = new UnmergedBranches(getRevisionComparator());
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java?rev=1696202&r1=1696201&r2=1696202&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStoreService.java Mon Aug 17 07:47:15 2015
@@ -364,7 +364,8 @@ public class DocumentNodeStoreService {
diffCachePercentage).
setCacheSegmentCount(cacheSegmentCount).
setCacheStackMoveDistance(cacheStackMoveDistance).
- offHeapCacheSize(offHeapCache * MB);
+ offHeapCacheSize(offHeapCache * MB).
+ setLeaseCheck(true /* OAK-2739: enabled by default */);
if (persistentCache != null && persistentCache.length() > 0) {
mkBuilder.setPersistentCache(persistentCache);
Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LeaseCheckDocumentStoreWrapper.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LeaseCheckDocumentStoreWrapper.java?rev=1696202&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LeaseCheckDocumentStoreWrapper.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LeaseCheckDocumentStoreWrapper.java Mon Aug 17 07:47:15 2015
@@ -0,0 +1,197 @@
+/*
+ * 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.jackrabbit.oak.plugins.document.util;
+
+import java.util.List;
+import java.util.Map;
+
+import org.apache.jackrabbit.oak.cache.CacheStats;
+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.DocumentStore;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Condition;
+import org.apache.jackrabbit.oak.plugins.document.UpdateOp.Key;
+import org.apache.jackrabbit.oak.plugins.document.cache.CacheInvalidationStats;
+
+/**
+ * Wrapper of another DocumentStore that does a lease check on any method
+ * invocation (read or update) and fails if the lease is not valid.
+ * <p>
+ * @see https://issues.apache.org/jira/browse/OAK-2739 for more details
+ */
+public final class LeaseCheckDocumentStoreWrapper implements DocumentStore {
+
+ private final DocumentStore delegate;
+ private final ClusterNodeInfo clusterNodeInfo;
+
+ public LeaseCheckDocumentStoreWrapper(final DocumentStore delegate,
+ final ClusterNodeInfo clusterNodeInfo) {
+ if (delegate==null) {
+ throw new IllegalArgumentException("delegate must not be null");
+ }
+ if (clusterNodeInfo==null) {
+ throw new IllegalArgumentException("clusterNodeInfo must not be null");
+ }
+ this.delegate = delegate;
+ this.clusterNodeInfo = clusterNodeInfo;
+ }
+
+ private final void performLeaseCheck() {
+ clusterNodeInfo.performLeaseCheck();
+ }
+
+ @Override
+ public final <T extends Document> T find(Collection<T> collection, String key) {
+ performLeaseCheck();
+ return delegate.find(collection, key);
+ }
+
+ @Override
+ public final <T extends Document> T find(Collection<T> collection, String key,
+ int maxCacheAge) {
+ performLeaseCheck();
+ return delegate.find(collection, key, maxCacheAge);
+ }
+
+ @Override
+ public final <T extends Document> List<T> query(Collection<T> collection,
+ String fromKey, String toKey, int limit) {
+ performLeaseCheck();
+ return delegate.query(collection, fromKey, toKey, limit);
+ }
+
+ @Override
+ public final <T extends Document> List<T> query(Collection<T> collection,
+ String fromKey, String toKey, String indexedProperty,
+ long startValue, int limit) {
+ performLeaseCheck();
+ return delegate.query(collection, fromKey, toKey, indexedProperty, startValue, limit);
+ }
+
+ @Override
+ public final <T extends Document> void remove(Collection<T> collection, String key) {
+ performLeaseCheck();
+ delegate.remove(collection, key);
+ }
+
+ @Override
+ public final <T extends Document> void remove(Collection<T> collection,
+ List<String> keys) {
+ performLeaseCheck();
+ delegate.remove(collection, keys);
+ }
+
+ @Override
+ public final <T extends Document> int remove(Collection<T> collection,
+ Map<String, Map<Key, Condition>> toRemove) {
+ performLeaseCheck();
+ return delegate.remove(collection, toRemove);
+ }
+
+ @Override
+ public final <T extends Document> boolean create(Collection<T> collection,
+ List<UpdateOp> updateOps) {
+ performLeaseCheck();
+ return delegate.create(collection, updateOps);
+ }
+
+ @Override
+ public final <T extends Document> void update(Collection<T> collection,
+ List<String> keys, UpdateOp updateOp) {
+ performLeaseCheck();
+ delegate.update(collection, keys, updateOp);
+ }
+
+ @Override
+ public final <T extends Document> T createOrUpdate(Collection<T> collection,
+ UpdateOp update) {
+ performLeaseCheck();
+ return delegate.createOrUpdate(collection, update);
+ }
+
+ @Override
+ public final <T extends Document> T findAndUpdate(Collection<T> collection,
+ UpdateOp update) {
+ performLeaseCheck();
+ return delegate.findAndUpdate(collection, update);
+ }
+
+ @Override
+ public final CacheInvalidationStats invalidateCache() {
+ performLeaseCheck();
+ return delegate.invalidateCache();
+ }
+
+ @Override
+ public final CacheInvalidationStats invalidateCache(Iterable<String> keys) {
+ performLeaseCheck();
+ return delegate.invalidateCache(keys);
+ }
+
+ @Override
+ public final <T extends Document> void invalidateCache(Collection<T> collection,
+ String key) {
+ performLeaseCheck();
+ delegate.invalidateCache(collection, key);
+ }
+
+ @Override
+ public final void dispose() {
+ // this is debatable whether or not a lease check should be done on dispose.
+ // I'd say the lease must still be valid as on dispose there could be
+ // stuff written to the document store which should only be done
+ // when the lease is valid
+ performLeaseCheck();
+ delegate.dispose();
+ }
+
+ @Override
+ public final <T extends Document> T getIfCached(Collection<T> collection,
+ String key) {
+ performLeaseCheck();
+ return delegate.getIfCached(collection, key);
+ }
+
+ @Override
+ public final void setReadWriteMode(String readWriteMode) {
+ performLeaseCheck();
+ delegate.setReadWriteMode(readWriteMode);
+ }
+
+ @Override
+ public final CacheStats getCacheStats() {
+ performLeaseCheck();
+ return delegate.getCacheStats();
+ }
+
+ @Override
+ public final Map<String, String> getMetadata() {
+ performLeaseCheck();
+ return delegate.getMetadata();
+ }
+
+ @Override
+ public long determineServerTimeDifferenceMillis() {
+ performLeaseCheck();
+ return delegate.determineServerTimeDifferenceMillis();
+ }
+
+}
Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/util/LeaseCheckDocumentStoreWrapper.java
------------------------------------------------------------------------------
svn:mime-type = text/plain
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDocumen...
Posted by Stefan Egli <st...@apache.org>.
MustFixTypoException:
On 18/08/15 11:14, "Stefan Egli" <egli@adobe.com on behalf of
stefanegli@apache.org> wrote:
>d) none of the above and Oak tries to rejoin the cluster and continues to
>function (in my view this will not result in unmanageable edge cases)
.. (in my view this *will* result in unmanageable edge cases) ..
Cheers,
Stefan
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Carsten Ziegeler <cz...@apache.org>.
I guess at least for the OSGi world, not having a repository service
should solve the problem nicely.
Carsten
Am 18.08.15 um 12:24 schrieb Michael Marth:
> I think option b) would be not so bad - maybe by starting a commit hook that denies any commit?
> (and screaming loudly in the logs)
>
>
>
>
> On 18/08/15 11:24, "Julian Reschke" <ju...@gmx.de> wrote:
>
>> On 2015-08-18 11:14, Stefan Egli wrote:
>>> On 18/08/15 10:57, "Julian Reschke" wrote:
>>> ...
>>> Hi Julian,
>>>
>>> The idea is indeed that if an instance fails to update the lease then it
>>> will be considered by other instances in the cluster as dead/crashed -
>>> even though it still continues to function. It is the only one that is
>>> able to detect such a situation. Imv letting the instance shutdown is at
>>> this moment the only reasonable reaction as upper level code might
>>> otherwise continue to function on the assumption it is part of the cluster
>>> - to which the other instances do not agree, the others consider this
>>> instance as died.
>>>
>>> So taking one step back: the lease becomes a vital part of the functioning
>>> of Oak indeed.
>>>
>>> I see three alternatives:
>>>
>>> a) Oak itself behaves fail-safe and does the System.exit (that¹s the path
>>> I have suggested for now)
>>>
>>> b) Oak does not do the System.exit but refuses to update anything towards
>>> the document store (thus just throws exceptions on each invocation) - and
>>> upper level code detects this situation (eg a Sling Health Check) and
>>> would do a System.exit based on how it is configured
>>>
>>> c) same as b) but upper level code does not do a System.exit (I¹m not sure
>>> if that makes sense - the instance is useless in such a situation)
>>>
>>> d) none of the above and Oak tries to rejoin the cluster and continues to
>>> function (in my view this will not result in unmanageable edge cases)
>>> ...
>>
>> Yes, we need to think about how to stop Oak in this case. However I do
>> not think that stopping the *VM* is something we can do here. Keep in
>> mind that there might be many other things running in the VM which have
>> nothing to do with the content repository.
>>
>> Best regards, Julian
--
Carsten Ziegeler
Adobe Research Switzerland
cziegeler@apache.org
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Michael Marth <mm...@adobe.com>.
I think option b) would be not so bad - maybe by starting a commit hook that denies any commit?
(and screaming loudly in the logs)
On 18/08/15 11:24, "Julian Reschke" <ju...@gmx.de> wrote:
>On 2015-08-18 11:14, Stefan Egli wrote:
>> On 18/08/15 10:57, "Julian Reschke" wrote:
>> ...
>> Hi Julian,
>>
>> The idea is indeed that if an instance fails to update the lease then it
>> will be considered by other instances in the cluster as dead/crashed -
>> even though it still continues to function. It is the only one that is
>> able to detect such a situation. Imv letting the instance shutdown is at
>> this moment the only reasonable reaction as upper level code might
>> otherwise continue to function on the assumption it is part of the cluster
>> - to which the other instances do not agree, the others consider this
>> instance as died.
>>
>> So taking one step back: the lease becomes a vital part of the functioning
>> of Oak indeed.
>>
>> I see three alternatives:
>>
>> a) Oak itself behaves fail-safe and does the System.exit (that¹s the path
>> I have suggested for now)
>>
>> b) Oak does not do the System.exit but refuses to update anything towards
>> the document store (thus just throws exceptions on each invocation) - and
>> upper level code detects this situation (eg a Sling Health Check) and
>> would do a System.exit based on how it is configured
>>
>> c) same as b) but upper level code does not do a System.exit (I¹m not sure
>> if that makes sense - the instance is useless in such a situation)
>>
>> d) none of the above and Oak tries to rejoin the cluster and continues to
>> function (in my view this will not result in unmanageable edge cases)
>> ...
>
>Yes, we need to think about how to stop Oak in this case. However I do
>not think that stopping the *VM* is something we can do here. Keep in
>mind that there might be many other things running in the VM which have
>nothing to do with the content repository.
>
>Best regards, Julian
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDocumen...
Posted by Julian Reschke <ju...@gmx.de>.
On 2015-08-18 11:14, Stefan Egli wrote:
> On 18/08/15 10:57, "Julian Reschke" wrote:
> ...
> Hi Julian,
>
> The idea is indeed that if an instance fails to update the lease then it
> will be considered by other instances in the cluster as dead/crashed -
> even though it still continues to function. It is the only one that is
> able to detect such a situation. Imv letting the instance shutdown is at
> this moment the only reasonable reaction as upper level code might
> otherwise continue to function on the assumption it is part of the cluster
> - to which the other instances do not agree, the others consider this
> instance as died.
>
> So taking one step back: the lease becomes a vital part of the functioning
> of Oak indeed.
>
> I see three alternatives:
>
> a) Oak itself behaves fail-safe and does the System.exit (that¹s the path
> I have suggested for now)
>
> b) Oak does not do the System.exit but refuses to update anything towards
> the document store (thus just throws exceptions on each invocation) - and
> upper level code detects this situation (eg a Sling Health Check) and
> would do a System.exit based on how it is configured
>
> c) same as b) but upper level code does not do a System.exit (I¹m not sure
> if that makes sense - the instance is useless in such a situation)
>
> d) none of the above and Oak tries to rejoin the cluster and continues to
> function (in my view this will not result in unmanageable edge cases)
> ...
Yes, we need to think about how to stop Oak in this case. However I do
not think that stopping the *VM* is something we can do here. Keep in
mind that there might be many other things running in the VM which have
nothing to do with the content repository.
Best regards, Julian
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Manfred Baedke <ma...@gmail.com>.
Whatever the long term solution will be: we need a short term solution
that doesn't kill an entire application server, so +1.
On 09.09.15 14:12, Stefan Egli wrote:
> Hi all,
>
> I'd like to follow up on the idea to restart DocumentNodeStore as a result
> of a lease failure [0]: I suggest we don't do that and instead just stop
> the oak-core bundle.
>
> After some prototyping and running into OAK-3373 [1] I'm no longer sure if
> restarting the DocumentNodeStore is a feasible path to go, esp in the
> short term. The problem encountered so far is that Observers cannot be
> easily switched from old to (restarted/)new store due to:
>
> * as pointed out by MichaelD they could have a backlog yet to process
> towards the old store - which they cannot access anymore as that one would
> be forcibly closed
> * there is not yet a proper way to switch from old to new ('reset') - esp
> is there a risk that there could be a gap (this part we might be able to
> fix though, not sure)
> * both above carry the risk that Observers miss some changes - something
> which would be unacceptable I guess.
>
> I think the more kiss approach would be to just forcibly close the
> DocumentNodeStore - or actually to stop the entire oak-core bundle - with
> appropriate errors logged so that the issue becomes clear. The instance
> would basically become unusable, mostly, but at least it would not be a
> System.exit.
>
> What do ppl think?
>
> Cheers,
> Stefan
> --
> [0] https://issues.apache.org/jira/browse/OAK-3250
> [1] https://issues.apache.org/jira/browse/OAK-3373
>
> On 18/08/15 16:45, "Stefan Egli" <eg...@adobe.com> wrote:
>
>> I've created OAK-3250 to follow up on the DocumentNodeStore-restart idea.
>>
>> Cheers,
>> Stefan
>> --
>> https://issues.apache.org/jira/browse/OAK-3250
>>
>> On 18/08/15 15:59, "Marcel Reutegger" <mr...@adobe.com> wrote:
>>
>>> On 18/08/15 15:38, "Stefan Egli" wrote:
>>>> On 18/08/15 13:43, "Marcel Reutegger" <mr...@adobe.com> wrote:
>>>>> On 18/08/15 11:14, "Stefan Egli" wrote:
>>>>>> b) Oak does not do the System.exit but refuses to update anything
>>>>>> towards
>>>>>> the document store (thus just throws exceptions on each invocation) -
>>>>>> and
>>>>>> upper level code detects this situation (eg a Sling Health Check) and
>>>>>> would do a System.exit based on how it is configured
>>>>>>
>>>>>> c) same as b) but upper level code does not do a System.exit (I¹m not
>>>>>> sure
>>>>>> if that makes sense - the instance is useless in such a situation)
>>>>> either b) or c) sounds reasonable to me.
>>>>>
>>>>> but if possible I'd like to avoid a System.exit(). would it be possible
>>>>> to detect this situation in the DocumentNodeStoreService and restart
>>>>> the DocumentNodeStore without the need to restart the JVM
>>>> Good point. Perhaps restarting DocumentNodeStore is a valid alternative
>>>> indeed. Is that feasible from a DocumentNodeStore point of view?
>>> it probably requires some changes to the DocumentNodeStore, because
>>> we want it to tear down without doing any of the cleanup it
>>> may otherwise perform. it must not release the cluster node info
>>> nor update pending _lastRevs, etc.
>>>
>>>> What would be the consequences of a restarted DocumentNodeStore?
>>> to the DocumentNodeStore it will look like it was killed and it will
>>> perform recovery (e.g. for the pending _lastRevs).
>>>
>>> Regards
>>> Marcel
>>>
>
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
My vote would also be (b) for the short-term. If we figure out a way to
properly restart the nodestore (c) we can still come back to that at a
later time.
Hence I've created https://issues.apache.org/jira/browse/OAK-3397 and
unless the list vetoes I'll follow up on that next.
Cheers,
Stefan
On 11/09/15 11:38, "Julian Sedding" <js...@gmail.com> wrote:
>My preference is (b), even though I think stopping the NodeStore
>service should be sufficient (it may not currently be sufficient, I
>don't know).
>
>Particularly, I believe that "trying harder" is detrimental to the
>overall stability of a cluster/topology. We are dealing with a
>possibly faulty instance, so who can decide that it is ok again after
>trying harder? The faulty instance itself?
>
>"Read-only" doesn't sound too useful either, because that may fool
>clients into thinking they are dealing with a "healthy" instance for
>longer than necessary and thus can lead to bigger issues downstream.
>
>I believe that "fail early and fail often" is the path to a stable
>cluster.
>
>Regards
>Julian
>
>On Thu, Sep 10, 2015 at 6:43 PM, Stefan Egli <st...@apache.org>
>wrote:
>> On 09/09/15 18:11, "Stefan Egli" <st...@apache.org> wrote:
>>
>>>On 09/09/15 18:01, "Stefan Egli" <st...@apache.org> wrote:
>>>
>>>>I think if the observers would all be 'OSGi-ified' then this could be
>>>>achieved. But currently eg the BackgroundObserver is just a pojo and
>>>>not
>>>>an osgi component (thus doesn't support any activate/deactivate method
>>>>hooks).
>>>
>>>.. I take that back - going via OsgiWhiteboard should work as desired -
>>>so
>>>perhaps implementing deactivate/activate methods in the
>>>(Background)Observer(s) would do the trick .. I'll give it a try ..
>>
>> ootb this wont work as the BackgroundObserver, as one example, is not an
>> OSGi component, so wont get any deactivate/activate calls atm. so to
>> achieve this, it would have to be properly OSGi-ified - something which
>> sounds like a bigger task and not only limited to this one class - which
>> means making DocumentNodeStore 'restart capable' sounds like a bigger
>>task
>> too and the question is indeed if it is worth while ('will it work?') or
>> if there are alternatives..
>>
>> which brings me back to the original question as to what should be done
>>in
>> case of a lease failure - to recap the options left (if System.exit is
>>not
>> one of them) are:
>>
>> a) 'go read-only': prevent writes by throwing exceptions from this
>>moment
>> until eternity
>>
>> b) 'stop oak': stop the oak-core bundle (prevent writes by throwing
>> exceptions for those still reaching out for the nodeStore)
>>
>> c) 'try harder': try to reactivate the lease - continue allowing writes
>>-
>> and make sure the next backgroundWrite has correctly updated the
>> 'unsavedLastRevisions' (cos others could have done a recover of this
>>node,
>> so unsavedLastRevisions contains superfluous stuff that must no longer
>>be
>> written). this would open the door for edge cases ('change of longer
>>time
>> window with multiple leaders') but perhaps is not entirely impossible...
>>
>> additionally/independently:
>>
>> * in all cases the discovery-lite descriptor should expose this lease
>> failure/partitioning situation - so that anyone can react who would like
>> to, esp should anyone no longer assume that the local instance is leader
>> or part of the cluster - and to support that optional Sling Health Check
>> which still does a System.exit :)
>>
>> * also, we should probably increase the lease thread's priority to
>>reduce
>> the likelihood of the lease timing out (same would be true for
>> discovery.impl's heartbeat thread)
>>
>>
>> * plus increasing the lease time from 1min to perhaps 5min as the
>>default
>> would also reduce the number of cases that hit problems dramatically
>>
>> wdyt?
>>
>> Cheers,
>> Stefan
>>
>>
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Julian Sedding <js...@gmail.com>.
My preference is (b), even though I think stopping the NodeStore
service should be sufficient (it may not currently be sufficient, I
don't know).
Particularly, I believe that "trying harder" is detrimental to the
overall stability of a cluster/topology. We are dealing with a
possibly faulty instance, so who can decide that it is ok again after
trying harder? The faulty instance itself?
"Read-only" doesn't sound too useful either, because that may fool
clients into thinking they are dealing with a "healthy" instance for
longer than necessary and thus can lead to bigger issues downstream.
I believe that "fail early and fail often" is the path to a stable cluster.
Regards
Julian
On Thu, Sep 10, 2015 at 6:43 PM, Stefan Egli <st...@apache.org> wrote:
> On 09/09/15 18:11, "Stefan Egli" <st...@apache.org> wrote:
>
>>On 09/09/15 18:01, "Stefan Egli" <st...@apache.org> wrote:
>>
>>>I think if the observers would all be 'OSGi-ified' then this could be
>>>achieved. But currently eg the BackgroundObserver is just a pojo and not
>>>an osgi component (thus doesn't support any activate/deactivate method
>>>hooks).
>>
>>.. I take that back - going via OsgiWhiteboard should work as desired - so
>>perhaps implementing deactivate/activate methods in the
>>(Background)Observer(s) would do the trick .. I'll give it a try ..
>
> ootb this wont work as the BackgroundObserver, as one example, is not an
> OSGi component, so wont get any deactivate/activate calls atm. so to
> achieve this, it would have to be properly OSGi-ified - something which
> sounds like a bigger task and not only limited to this one class - which
> means making DocumentNodeStore 'restart capable' sounds like a bigger task
> too and the question is indeed if it is worth while ('will it work?') or
> if there are alternatives..
>
> which brings me back to the original question as to what should be done in
> case of a lease failure - to recap the options left (if System.exit is not
> one of them) are:
>
> a) 'go read-only': prevent writes by throwing exceptions from this moment
> until eternity
>
> b) 'stop oak': stop the oak-core bundle (prevent writes by throwing
> exceptions for those still reaching out for the nodeStore)
>
> c) 'try harder': try to reactivate the lease - continue allowing writes -
> and make sure the next backgroundWrite has correctly updated the
> 'unsavedLastRevisions' (cos others could have done a recover of this node,
> so unsavedLastRevisions contains superfluous stuff that must no longer be
> written). this would open the door for edge cases ('change of longer time
> window with multiple leaders') but perhaps is not entirely impossible...
>
> additionally/independently:
>
> * in all cases the discovery-lite descriptor should expose this lease
> failure/partitioning situation - so that anyone can react who would like
> to, esp should anyone no longer assume that the local instance is leader
> or part of the cluster - and to support that optional Sling Health Check
> which still does a System.exit :)
>
> * also, we should probably increase the lease thread's priority to reduce
> the likelihood of the lease timing out (same would be true for
> discovery.impl's heartbeat thread)
>
>
> * plus increasing the lease time from 1min to perhaps 5min as the default
> would also reduce the number of cases that hit problems dramatically
>
> wdyt?
>
> Cheers,
> Stefan
>
>
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
On 10/09/15 18:43, "Stefan Egli" <st...@apache.org> wrote:
>additionally/independently:
>
>[...]
>
>* also, we should probably increase the lease thread's priority to reduce
>the likelihood of the lease timing out (same would be true for
>discovery.impl's heartbeat thread)
>
>* plus increasing the lease time from 1min to perhaps 5min as the default
>would also reduce the number of cases that hit problems dramatically
FYI: Put these suggested improvements into:
https://issues.apache.org/jira/browse/OAK-3398
most noteworthy: I suggest to increase the lease timeout by default to
120sec. (not 5min, I think that's too much)
Cheers,
Stefan
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
On 09/09/15 18:11, "Stefan Egli" <st...@apache.org> wrote:
>On 09/09/15 18:01, "Stefan Egli" <st...@apache.org> wrote:
>
>>I think if the observers would all be 'OSGi-ified' then this could be
>>achieved. But currently eg the BackgroundObserver is just a pojo and not
>>an osgi component (thus doesn't support any activate/deactivate method
>>hooks).
>
>.. I take that back - going via OsgiWhiteboard should work as desired - so
>perhaps implementing deactivate/activate methods in the
>(Background)Observer(s) would do the trick .. I'll give it a try ..
ootb this wont work as the BackgroundObserver, as one example, is not an
OSGi component, so wont get any deactivate/activate calls atm. so to
achieve this, it would have to be properly OSGi-ified - something which
sounds like a bigger task and not only limited to this one class - which
means making DocumentNodeStore 'restart capable' sounds like a bigger task
too and the question is indeed if it is worth while ('will it work?') or
if there are alternatives..
which brings me back to the original question as to what should be done in
case of a lease failure - to recap the options left (if System.exit is not
one of them) are:
a) 'go read-only': prevent writes by throwing exceptions from this moment
until eternity
b) 'stop oak': stop the oak-core bundle (prevent writes by throwing
exceptions for those still reaching out for the nodeStore)
c) 'try harder': try to reactivate the lease - continue allowing writes -
and make sure the next backgroundWrite has correctly updated the
'unsavedLastRevisions' (cos others could have done a recover of this node,
so unsavedLastRevisions contains superfluous stuff that must no longer be
written). this would open the door for edge cases ('change of longer time
window with multiple leaders') but perhaps is not entirely impossible...
additionally/independently:
* in all cases the discovery-lite descriptor should expose this lease
failure/partitioning situation - so that anyone can react who would like
to, esp should anyone no longer assume that the local instance is leader
or part of the cluster - and to support that optional Sling Health Check
which still does a System.exit :)
* also, we should probably increase the lease thread's priority to reduce
the likelihood of the lease timing out (same would be true for
discovery.impl's heartbeat thread)
* plus increasing the lease time from 1min to perhaps 5min as the default
would also reduce the number of cases that hit problems dramatically
wdyt?
Cheers,
Stefan
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
On 09/09/15 18:01, "Stefan Egli" <st...@apache.org> wrote:
>I think if the observers would all be 'OSGi-ified' then this could be
>achieved. But currently eg the BackgroundObserver is just a pojo and not
>an osgi component (thus doesn't support any activate/deactivate method
>hooks).
.. I take that back - going via OsgiWhiteboard should work as desired - so
perhaps implementing deactivate/activate methods in the
(Background)Observer(s) would do the trick .. I'll give it a try ..
Cheers,
Stefan
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
Hi,
On 09/09/15 17:39, "Marcel Reutegger" <mr...@adobe.com> wrote:
>>* as pointed out by MichaelD they could have a backlog yet to process
>>towards the old store - which they cannot access anymore as that one
>>would
>>be forcibly closed
>
>in my view, those observers should be unregistered from the store before
>it is shut down and any backlog cleared, i.e. it will be lost.
yes they do get unregistered right away indeed - but atm there's no handle
as to prevent eg the BackgroundObserver from still having entries in the
queue and continuing to process them. so those queued entries will indeed
fail as the store is closed.
>>* there is not yet a proper way to switch from old to new ('reset') - esp
>>is there a risk that there could be a gap (this part we might be able to
>>fix though, not sure)
>
>I don't see a requirement for this. if you restart the entire stack you
>will also have a gap.
the difference is perhaps that if you restart the stack this is done as an
explicit admin operation, knowingly. While as what we're trying to achieve
here is something automated, 'under the hood', which has a different
quality requirement imv.
>>* both above carry the risk that Observers miss some changes - something
>>which would be unacceptable I guess.
>
>same as above. I don't think observers must survive a node store restart.
>I even think it is wrong. Every client of the node store should be
>restarted in that case, including Observers.
I think if the observers would all be 'OSGi-ified' then this could be
achieved. But currently eg the BackgroundObserver is just a pojo and not
an osgi component (thus doesn't support any activate/deactivate method
hooks).
Cheers,
Stefan
RE: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Rob Ryan <rr...@adobe.com>.
This starts to sound like Oak should have an explicit interface to announce the situation to clients, allowing them to implement their own policy about whether to attempt a restart or not.
-Rob
-----Original Message-----
From: Marcel Reutegger [mailto:mreutegg@adobe.com]
Sent: Wednesday, September 09, 2015 8:39 AM
To: oak-dev@jackrabbit.apache.org
Subject: Re: System.exit()???? , was: svn commit: r1696202 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document: ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java DocumentNodeStoreService.java util/LeaseCheckDoc...
Hi,
On 09/09/15 14:12, "Stefan Egli" wrote:
>After some prototyping and running into OAK-3373 [1] I'm no longer sure
>if restarting the DocumentNodeStore is a feasible path to go, esp in
>the short term. The problem encountered so far is that Observers cannot
>be easily switched from old to (restarted/)new store due to:
>
> * as pointed out by MichaelD they could have a backlog yet to process
>towards the old store - which they cannot access anymore as that one
>would be forcibly closed
in my view, those observers should be unregistered from the store before it is shut down and any backlog cleared, i.e. it will be lost.
> * there is not yet a proper way to switch from old to new ('reset') -
>esp is there a risk that there could be a gap (this part we might be
>able to fix though, not sure)
I don't see a requirement for this. if you restart the entire stack you will also have a gap.
> * both above carry the risk that Observers miss some changes -
>something which would be unacceptable I guess.
same as above. I don't think observers must survive a node store restart.
I even think it is wrong. Every client of the node store should be restarted in that case, including Observers.
Regards
Marcel
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Marcel Reutegger <mr...@adobe.com>.
Hi,
On 09/09/15 14:12, "Stefan Egli" wrote:
>After some prototyping and running into OAK-3373 [1] I'm no longer sure if
>restarting the DocumentNodeStore is a feasible path to go, esp in the
>short term. The problem encountered so far is that Observers cannot be
>easily switched from old to (restarted/)new store due to:
>
> * as pointed out by MichaelD they could have a backlog yet to process
>towards the old store - which they cannot access anymore as that one would
>be forcibly closed
in my view, those observers should be unregistered from the store before
it is shut down and any backlog cleared, i.e. it will be lost.
> * there is not yet a proper way to switch from old to new ('reset') - esp
>is there a risk that there could be a gap (this part we might be able to
>fix though, not sure)
I don't see a requirement for this. if you restart the entire stack you
will also have a gap.
> * both above carry the risk that Observers miss some changes - something
>which would be unacceptable I guess.
same as above. I don't think observers must survive a node store restart.
I even think it is wrong. Every client of the node store should be
restarted in that case, including Observers.
Regards
Marcel
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
Hi all,
I'd like to follow up on the idea to restart DocumentNodeStore as a result
of a lease failure [0]: I suggest we don't do that and instead just stop
the oak-core bundle.
After some prototyping and running into OAK-3373 [1] I'm no longer sure if
restarting the DocumentNodeStore is a feasible path to go, esp in the
short term. The problem encountered so far is that Observers cannot be
easily switched from old to (restarted/)new store due to:
* as pointed out by MichaelD they could have a backlog yet to process
towards the old store - which they cannot access anymore as that one would
be forcibly closed
* there is not yet a proper way to switch from old to new ('reset') - esp
is there a risk that there could be a gap (this part we might be able to
fix though, not sure)
* both above carry the risk that Observers miss some changes - something
which would be unacceptable I guess.
I think the more kiss approach would be to just forcibly close the
DocumentNodeStore - or actually to stop the entire oak-core bundle - with
appropriate errors logged so that the issue becomes clear. The instance
would basically become unusable, mostly, but at least it would not be a
System.exit.
What do ppl think?
Cheers,
Stefan
--
[0] https://issues.apache.org/jira/browse/OAK-3250
[1] https://issues.apache.org/jira/browse/OAK-3373
On 18/08/15 16:45, "Stefan Egli" <eg...@adobe.com> wrote:
>I've created OAK-3250 to follow up on the DocumentNodeStore-restart idea.
>
>Cheers,
>Stefan
>--
>https://issues.apache.org/jira/browse/OAK-3250
>
>On 18/08/15 15:59, "Marcel Reutegger" <mr...@adobe.com> wrote:
>
>>On 18/08/15 15:38, "Stefan Egli" wrote:
>>>On 18/08/15 13:43, "Marcel Reutegger" <mr...@adobe.com> wrote:
>>>>On 18/08/15 11:14, "Stefan Egli" wrote:
>>>>>b) Oak does not do the System.exit but refuses to update anything
>>>>>towards
>>>>>the document store (thus just throws exceptions on each invocation) -
>>>>>and
>>>>>upper level code detects this situation (eg a Sling Health Check) and
>>>>>would do a System.exit based on how it is configured
>>>>>
>>>>>c) same as b) but upper level code does not do a System.exit (I¹m not
>>>>>sure
>>>>>if that makes sense - the instance is useless in such a situation)
>>>>
>>>>either b) or c) sounds reasonable to me.
>>>>
>>>>but if possible I'd like to avoid a System.exit(). would it be possible
>>>>to detect this situation in the DocumentNodeStoreService and restart
>>>>the DocumentNodeStore without the need to restart the JVM
>>>
>>>Good point. Perhaps restarting DocumentNodeStore is a valid alternative
>>>indeed. Is that feasible from a DocumentNodeStore point of view?
>>
>>it probably requires some changes to the DocumentNodeStore, because
>>we want it to tear down without doing any of the cleanup it
>>may otherwise perform. it must not release the cluster node info
>>nor update pending _lastRevs, etc.
>>
>>> What would be the consequences of a restarted DocumentNodeStore?
>>
>>to the DocumentNodeStore it will look like it was killed and it will
>>perform recovery (e.g. for the pending _lastRevs).
>>
>>Regards
>> Marcel
>>
>
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <eg...@adobe.com>.
I've created OAK-3250 to follow up on the DocumentNodeStore-restart idea.
Cheers,
Stefan
--
https://issues.apache.org/jira/browse/OAK-3250
On 18/08/15 15:59, "Marcel Reutegger" <mr...@adobe.com> wrote:
>On 18/08/15 15:38, "Stefan Egli" wrote:
>>On 18/08/15 13:43, "Marcel Reutegger" <mr...@adobe.com> wrote:
>>>On 18/08/15 11:14, "Stefan Egli" wrote:
>>>>b) Oak does not do the System.exit but refuses to update anything
>>>>towards
>>>>the document store (thus just throws exceptions on each invocation) -
>>>>and
>>>>upper level code detects this situation (eg a Sling Health Check) and
>>>>would do a System.exit based on how it is configured
>>>>
>>>>c) same as b) but upper level code does not do a System.exit (I¹m not
>>>>sure
>>>>if that makes sense - the instance is useless in such a situation)
>>>
>>>either b) or c) sounds reasonable to me.
>>>
>>>but if possible I'd like to avoid a System.exit(). would it be possible
>>>to detect this situation in the DocumentNodeStoreService and restart
>>>the DocumentNodeStore without the need to restart the JVM
>>
>>Good point. Perhaps restarting DocumentNodeStore is a valid alternative
>>indeed. Is that feasible from a DocumentNodeStore point of view?
>
>it probably requires some changes to the DocumentNodeStore, because
>we want it to tear down without doing any of the cleanup it
>may otherwise perform. it must not release the cluster node info
>nor update pending _lastRevs, etc.
>
>> What would be the consequences of a restarted DocumentNodeStore?
>
>to the DocumentNodeStore it will look like it was killed and it will
>perform recovery (e.g. for the pending _lastRevs).
>
>Regards
> Marcel
>
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Marcel Reutegger <mr...@adobe.com>.
On 18/08/15 15:38, "Stefan Egli" wrote:
>On 18/08/15 13:43, "Marcel Reutegger" <mr...@adobe.com> wrote:
>>On 18/08/15 11:14, "Stefan Egli" wrote:
>>>b) Oak does not do the System.exit but refuses to update anything
>>>towards
>>>the document store (thus just throws exceptions on each invocation) -
>>>and
>>>upper level code detects this situation (eg a Sling Health Check) and
>>>would do a System.exit based on how it is configured
>>>
>>>c) same as b) but upper level code does not do a System.exit (I¹m not
>>>sure
>>>if that makes sense - the instance is useless in such a situation)
>>
>>either b) or c) sounds reasonable to me.
>>
>>but if possible I'd like to avoid a System.exit(). would it be possible
>>to detect this situation in the DocumentNodeStoreService and restart
>>the DocumentNodeStore without the need to restart the JVM
>
>Good point. Perhaps restarting DocumentNodeStore is a valid alternative
>indeed. Is that feasible from a DocumentNodeStore point of view?
it probably requires some changes to the DocumentNodeStore, because
we want it to tear down without doing any of the cleanup it
may otherwise perform. it must not release the cluster node info
nor update pending _lastRevs, etc.
> What would be the consequences of a restarted DocumentNodeStore?
to the DocumentNodeStore it will look like it was killed and it will
perform recovery (e.g. for the pending _lastRevs).
Regards
Marcel
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Stefan Egli <st...@apache.org>.
On 18/08/15 13:43, "Marcel Reutegger" <mr...@adobe.com> wrote:
>On 18/08/15 11:14, "Stefan Egli" wrote:
>>b) Oak does not do the System.exit but refuses to update anything towards
>>the document store (thus just throws exceptions on each invocation) - and
>>upper level code detects this situation (eg a Sling Health Check) and
>>would do a System.exit based on how it is configured
>>
>>c) same as b) but upper level code does not do a System.exit (I¹m not
>>sure
>>if that makes sense - the instance is useless in such a situation)
>
>either b) or c) sounds reasonable to me.
>
>but if possible I'd like to avoid a System.exit(). would it be possible
>to detect this situation in the DocumentNodeStoreService and restart
>the DocumentNodeStore without the need to restart the JVM
Good point. Perhaps restarting DocumentNodeStore is a valid alternative
indeed. Is that feasible from a DocumentNodeStore point of view? What
would be the consequences of a restarted DocumentNodeStore?
>or would this
>lead to an illegal state from a discovery POV?
Have to think through the scenarios but perhaps this is fine (I was indeed
initially under the assumption that it would not be fine, but that might
have been wrong). The important bit is that any topology-related activity
stops - and this can be achieved by sending TOPOLOGY_CHANGING (which in
turn could be achieved by setting the own instance into 'deactivating'
state in the discovery-lite-descriptor) and only coming back with
TOPOLOGY_CHANGED once the restart would be settled and the local instance
is back in the cluster with a valid, new lease.
Cheers,
Stefan
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDoc...
Posted by Marcel Reutegger <mr...@adobe.com>.
On 18/08/15 11:14, "Stefan Egli" wrote:
>b) Oak does not do the System.exit but refuses to update anything towards
>the document store (thus just throws exceptions on each invocation) - and
>upper level code detects this situation (eg a Sling Health Check) and
>would do a System.exit based on how it is configured
>
>c) same as b) but upper level code does not do a System.exit (I¹m not sure
>if that makes sense - the instance is useless in such a situation)
either b) or c) sounds reasonable to me.
but if possible I'd like to avoid a System.exit(). would it be possible
to detect this situation in the DocumentNodeStoreService and restart
the DocumentNodeStore without the need to restart the JVM or would this
lead to an illegal state from a discovery POV?
Regards
Marcel
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDocumen...
Posted by Stefan Egli <st...@apache.org>.
On 18/08/15 10:57, "Julian Reschke" wrote:
>On 2015-08-17 09:47, stefanegli@apache.org wrote:
>> Author: stefanegli
>> + @Override
>> + public void run() {
>> + System.exit(-1);
>> + }
>>...
>
>I'm a bit concerned (and that's an understatement) that OAK is now
>calling System.exit. Detecting a serious problem - good. Stopping the
>content repository - probably good, at least for write operations? But
>stopping the whole VM, no matter what else it runs? Seriously?
Hi Julian,
The idea is indeed that if an instance fails to update the lease then it
will be considered by other instances in the cluster as dead/crashed -
even though it still continues to function. It is the only one that is
able to detect such a situation. Imv letting the instance shutdown is at
this moment the only reasonable reaction as upper level code might
otherwise continue to function on the assumption it is part of the cluster
- to which the other instances do not agree, the others consider this
instance as died.
So taking one step back: the lease becomes a vital part of the functioning
of Oak indeed.
I see three alternatives:
a) Oak itself behaves fail-safe and does the System.exit (that¹s the path
I have suggested for now)
b) Oak does not do the System.exit but refuses to update anything towards
the document store (thus just throws exceptions on each invocation) - and
upper level code detects this situation (eg a Sling Health Check) and
would do a System.exit based on how it is configured
c) same as b) but upper level code does not do a System.exit (I¹m not sure
if that makes sense - the instance is useless in such a situation)
d) none of the above and Oak tries to rejoin the cluster and continues to
function (in my view this will not result in unmanageable edge cases)
Cheers,
Stefan
Re: System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDocumentStoreWrapper.java
Posted by Bertrand Delacretaz <bd...@apache.org>.
Hi,
On Tue, Aug 18, 2015 at 10:57 AM, Julian Reschke <ju...@gmx.de> wrote:
> I'm a bit concerned (and that's an understatement) that OAK is now calling
> System.exit. Detecting a serious problem - good. Stopping the content
> repository - probably good, at least for write operations? But stopping the
> whole VM, no matter what else it runs? Seriously?..
+1
-Bertrand
System.exit()???? , was: svn commit: r1696202 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document:
ClusterNodeInfo.java DocumentMK.java DocumentNodeStore.java
DocumentNodeStoreService.java util/LeaseCheckDocumentStoreWrapper.java
Posted by Julian Reschke <ju...@gmx.de>.
On 2015-08-17 09:47, stefanegli@apache.org wrote:
> Author: stefanegli
> Date: Mon Aug 17 07:47:15 2015
> New Revision: 1696202
>
> URL: http://svn.apache.org/r1696202
> Log:
> OAK-2739 : lease check introduced : by default there's now a check active which assures the local lease is valid upon every action done towards the DocumentStore
> ...
> + // OAK-2739 : when the lease is not current, we must stop
> + // the instance immediately to avoid any cluster inconsistency
> + final String errorMsg = "performLeaseCheck: this instance failed to update the lease in time "
> + + "(leaseEndTime: "+leaseEndTime+", now: "+now+", leaseTime: "+leaseTime+") "
> + + "and is thus no longer eligible for taking part in the cluster. Shutting down NOW!";
> + LOG.error(errorMsg);
> +
> + // now here comes the thing: we should a) call System.exit in a separate thread
> + // to avoid any deadlock when calling from eg within the shutdown hook
> + // AND b) we should not call system.exit hundred times.
> + // so for b) we use 'systemExitTriggered' to avoid calling it over and over
> + // BUT it doesn't have to be 100% ensured that system.exit is called only once.
> + // it is fine if it gets called once, twice - but just not hundred times.
> + // which is a long way of saying: volatile is fine here - and the 'if' too
> + if (!systemExitTriggered) {
> + systemExitTriggered = true;
> + final Runnable r = new Runnable() {
> +
> + @Override
> + public void run() {
> + System.exit(-1);
> + }
> +
> + };
> + final Thread th = new Thread(r, "FailedLeaseCheckShutdown-Thread");
> + th.setDaemon(true);
> + th.start();
> + }
> + throw new AssertionError(errorMsg);
> + }
> +
> ...
Hi everybody,
I'm a bit concerned (and that's an understatement) that OAK is now
calling System.exit. Detecting a serious problem - good. Stopping the
content repository - probably good, at least for write operations? But
stopping the whole VM, no matter what else it runs? Seriously?
Best regards, Julian