You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-dev@jackrabbit.apache.org by "mbaedke (via GitHub)" <gi...@apache.org> on 2024/11/26 14:33:10 UTC
[PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
mbaedke opened a new pull request, #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877
…vRecovery executions slowing down startup
Introduced the system variable oak.syncRecoveryTimeout to limit the duration of a self recovery at startup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2501405274
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [66.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2520482177
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [61.5% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2514263460
Minor changes:
1. simplify sys property access
2. use custom clock used by LRA
3. log in ISO format and only if deadline changed
4. add logging fot the case above
~~~
diff --git a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
index 0418751982..caa88e1b9a 100644
--- a/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
+++ b/oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java
@@ -29,11 +29,11 @@ import static org.apache.jackrabbit.oak.plugins.document.util.Utils.PROPERTY_OR_
import static org.apache.jackrabbit.oak.plugins.document.util.Utils.isCommitted;
import static org.apache.jackrabbit.oak.plugins.document.util.Utils.resolveCommitRevision;
-import java.text.SimpleDateFormat;
+import java.time.LocalDateTime;
+import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
-import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
@@ -82,7 +82,8 @@ public class LastRevRecoveryAgent {
private final Consumer<Integer> afterRecovery;
- private static final SystemPropertySupplier<Long> SYNC_RECOVERY_TIMEOUT_MILLIS = SystemPropertySupplier.create("oak.syncRecoveryTimeoutMillis", Long.MAX_VALUE);
+ private static final long SYNC_RECOVERY_TIMEOUT_MILLIS =
+ SystemPropertySupplier.create("oak.syncRecoveryTimeoutMillis", Long.MAX_VALUE).get();
private static final long LOGINTERVALMS = TimeUnit.MINUTES.toMillis(1);
@@ -272,11 +273,19 @@ public class LastRevRecoveryAgent {
ClusterNodeInfoDocument nodeInfo = missingLastRevUtil.getClusterNodeInfo(clusterId);
if (nodeInfo != null && nodeInfo.isActive()) {
deadline = nodeInfo.getLeaseEndTime() - ClusterNodeInfo.DEFAULT_LEASE_FAILURE_MARGIN_MILLIS;
+ log.info("Deadline for synchronous recovery is {}.",
+ LocalDateTime.ofEpochSecond(deadline, 0, ZoneOffset.UTC));
}
- long now = System.currentTimeMillis();
- if (Long.MAX_VALUE - SYNC_RECOVERY_TIMEOUT_MILLIS.get() > now) {
- deadline = Math.min(deadline, now + SYNC_RECOVERY_TIMEOUT_MILLIS.get());
- log.info("Adjusted deadline for synchronous recovery. New deadline is {}", SimpleDateFormat.getDateTimeInstance().format(new Date(deadline)));
+ long now = revisionContext.getClock().millis();
+ // defensive: make sure we don't get an overflow below
+ if (Long.MAX_VALUE - SYNC_RECOVERY_TIMEOUT_MILLIS > now) {
+ long prevDeadline = deadline;
+ deadline = Math.min(deadline, now + SYNC_RECOVERY_TIMEOUT_MILLIS);
+ if (deadline != prevDeadline) {
+ log.info("Adjusted deadline for synchronous recovery from {} to {}.",
+ LocalDateTime.ofEpochSecond(prevDeadline, 0, ZoneOffset.UTC),
+ LocalDateTime.ofEpochSecond(deadline, 0, ZoneOffset.UTC));
+ }
}
}
~~~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2514279556
haven't thought much about impact of this but just wanted to state (the obvious) that we'd need to be careful to not cause higher cluster IDs to be used than previously. The more cluster IDs that have ever been used in a deployment, the higher some cost (RevisionVector getting larger, some operations iterating over all (ever existing) cluster IDs)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2514383220
well, it's opt-in...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on code in PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#discussion_r1867485915
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java:
##########
@@ -268,6 +273,11 @@ public int recover(final Iterable<NodeDocument> suspects,
if (nodeInfo != null && nodeInfo.isActive()) {
deadline = nodeInfo.getLeaseEndTime() - ClusterNodeInfo.DEFAULT_LEASE_FAILURE_MARGIN_MILLIS;
}
+ long now = System.currentTimeMillis();
+ if (Long.MAX_VALUE - SYNC_RECOVERY_TIMEOUT_MILLIS.get() > now) {
+ deadline = Math.min(deadline, now + SYNC_RECOVERY_TIMEOUT_MILLIS.get());
+ log.info("Adjusted deadline for synchronous recovery. New deadline is {}", SimpleDateFormat.getDateTimeInstance().format(new Date(deadline)));
+ }
Review Comment:
a) Should not use the default locale.
b) Maybe also log the previous deadline?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "mbaedke (via GitHub)" <gi...@apache.org>.
mbaedke merged PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2503758413
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [66.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2514324905
@stefan-egli - that sort of is the point. It allows the user to trade conservative use of clusterIds with shorter startup time (which, in our case, was seen at around 10min when the server had been crashed and stayed off for a longer period of time)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2503746194
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [66.7% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2503815895
## [](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877) **Quality Gate passed**
Issues
 [2 New issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [57.1% Coverage on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=org.apache.jackrabbit%3Ajackrabbit-oak&pullRequest=1877)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "reschke (via GitHub)" <gi...@apache.org>.
reschke commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2541135477
I recommend going ahead with this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "joerghoh (via GitHub)" <gi...@apache.org>.
joerghoh commented on code in PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#discussion_r1858750694
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java:
##########
@@ -79,6 +80,8 @@ public class LastRevRecoveryAgent {
private final Consumer<Integer> afterRecovery;
+ private static final SystemPropertySupplier<Long> SYNC_RECEVERY_TIMEOUT = SystemPropertySupplier.create("oak.syncRecoveryTimeout", Long.MAX_VALUE);
Review Comment:
would a unit make that property more understandable?
```suggestion
private static final SystemPropertySupplier<Long> SYNC_RECEVERY_TIMEOUT = SystemPropertySupplier.create("oak.syncRecoveryTimeoutMs", Long.MAX_VALUE);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "mbaedke (via GitHub)" <gi...@apache.org>.
mbaedke commented on code in PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#discussion_r1860377810
##########
oak-store-document/src/main/java/org/apache/jackrabbit/oak/plugins/document/LastRevRecoveryAgent.java:
##########
@@ -79,6 +80,8 @@ public class LastRevRecoveryAgent {
private final Consumer<Integer> afterRecovery;
+ private static final SystemPropertySupplier<Long> SYNC_RECEVERY_TIMEOUT = SystemPropertySupplier.create("oak.syncRecoveryTimeout", Long.MAX_VALUE);
Review Comment:
Yes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] OAK-11284: Greedy Reuse of cluster IDs may lead to synchronous LastRe… [jackrabbit-oak]
Posted by "stefan-egli (via GitHub)" <gi...@apache.org>.
stefan-egli commented on PR #1877:
URL: https://github.com/apache/jackrabbit-oak/pull/1877#issuecomment-2514344026
right, so this needs to be rolled out with care
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: oak-dev-unsubscribe@jackrabbit.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org