You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by GitBox <gi...@apache.org> on 2021/06/07 14:16:06 UTC
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh opened a new pull request #18: SLING-10418 implement a retry for the application of the statements
joerghoh opened a new pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] cziegeler commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
cziegeler commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857360154
I still don't understand why we need to apply this blindly to every operation. creating a path seems to be the critical one. So why not just handle this?
In addition, as mentioned in the comment - logging the same exception/error several times is not user friendly. If the error is transient and a retry is solving the problem, don't log any error/exception because from a user perspective there is no error. If the error is not transient, log a single error/exception, because again from a user perspective there is only one error not N.,
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-855973694
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='55.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [55.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857479263
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='53.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [53.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856533401
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857440053
It is a transient problem, because mixins are also stored as part of the repo:
```
Caused by: org.apache.jackrabbit.oak.api.CommitFailedException: OakState0001: Unresolved conflicts in /jcr:system/jcr:nodeTypes/dam:cfVariationNode
```
and can fail due to concurrency (as indicated by this OakState0001 exception). Same as path creation, user creation, ACL setup etc. Moving the retries into these specific operations doesn't make much sense to me, as it would duplicate a lot of code with no benefit.
There might be root causes where retry doesn't make sense because the operation will always fail (for example setting an ACL to a non-existing path). Avoiding to retry these is work because you need to review the different types of exceptions to understand if they indicate a transient or permanent failure, and if a retry makes sense.
Of course we can do that, but I consider the benefit very limited, because it would save ~6 seconds (retries after 1,2 and 3 seconds) until the repoinit operation finally fails. Also situations like this (incorrect repo init statements) should never the normal case, while concurrency issues are more likely to happen. The case mentioned above with perfectly valid rep init statements led to startup failures of a more than dozen cases.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648079827
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +124,58 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
Review comment:
That is part of the Builder[1]. All repoinit operations share the same (hardcoded) retry settings. I don't see why it should be configurable in a more granular fashion.
[1] https://github.com/apache/sling-org-apache-sling-jcr-repoinit/blob/feature/SLING-10418-retry/src/main/java/org/apache/sling/jcr/repoinit/impl/RetryableOperation.java#L136
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856042130
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r646639611
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RetryableOperation.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.sling.jcr.repoinit.impl;
+
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A simple implementation of retryable operations.
+ * Use the builder class to create an instance of it.
+ *
+ */
+public class RetryableOperation {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RetryableOperation.class);
+
+ int backoffBase;
+ int maxRetries;
+ int jitter;
+
+ int retryCount = 0;
+
+ RetryableOperation(int backoff, int maxRetries, int jitter) {
+ this.backoffBase = backoff;
+ this.maxRetries = maxRetries;
+ this.jitter = jitter;
+ }
+ /**
+ * Execute the operation with the defined retry until it returns true or
+ * the retry aborts; in case the operation is retried, a log message is logged on INFO with the
+ * provided logMessage and the current number of retries
+ * @param operation
+ * @param logMessage the log message
+ * @return true if the supplier was eventually successful, false if it failed despite all retries
+ */
+ public boolean apply(Supplier<Boolean> operation, String logMessage) {
+
+ boolean successful = false;
+ successful = operation.get();
+ while (! successful && retryCount < maxRetries) {
+ retryCount++;
+ LOG.info("%s (retry %d/%d)", logMessage, retryCount, maxRetries);
+ delay(retryCount);
+ successful = operation.get();
+ }
+ return successful;
+ }
+
+ private void delay(int retryCount) {
+
+ int j = (int) (Math.random() * (jitter));
+ int delayInMilis = (backoffBase * retryCount) + j;
+ try {
+ TimeUnit.MILLISECONDS.sleep(delayInMilis);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+
+
+
+ public static class Builder {
+
+ int exponentialBackoff = 1000; // default
+ int maxRetries = 3; // default
+ int jitter = 200;
+
+ /**
+ * The backoff time
+ * @param msec backoff time in miliseconds
+ * @return the builder
+ */
+ Builder withBackoffBase(int msec) {
+ exponentialBackoff = msec;
+ return this;
+ }
+
+ /**
+ * Configures the number of retries;
+ * @param retries number of retries
+ * @return the builder
+ */
+ Builder withMaxRetries(int retries) {
+ this.maxRetries= retries;
+ return this;
+ }
+
+ /**
+ * configures the jitter
+ * @param msec the jitter in miliseconds
+ * @return the builder
+ */
+ Builder withJitter(int msec) {
Review comment:
Same as above, I would call this `withJitterMsec`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857593701
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='59.2%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [59.2% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857048382
The security hotspot issue in Sonar is bogus, because I want that randomness just to add some random delay, not for any type of security.
@bdelacretaz @cziegeler any objections, otherwise I would merge.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r646661450
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +131,47 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws Exception {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBase(1000).withMaxRetries(3).build();
+ boolean successful = retry.apply(() -> {
+ try {
+ processor.apply(session, ops);
+ session.save();
+ return true;
+ } catch (RepositoryException e) {
+ log.error("(temporarily) failed to apply repoinit operations",e);
Review comment:
Yes, that is possible, but I don't think it makes sense to suppress it, because we are never sure that it will always the same reason and the same exception. It's completely ok if the different RepositoryExceptions are thrown.
Anyway, I don't expect this situation to happen that often, as we have seen it only in rare circumstances. So that's for me an acceptable logging overhead.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857506599
Ok, ok, two against one, I'll reluctantly accept the hardcoded 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856042130
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648089422
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +124,58 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
Review comment:
I can make it configurable on the OSGI level, but I still question the need to actually do that. Can't we just stick with the hardcoded value for now and see if that's good enough? If we need to be more flexible, we add that feature later on.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] enapps-enorman commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
enapps-enorman commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857269436
> The security hotspot issue in Sonar is bogus, because I want that randomness just to add some random delay, not for any type of security.
>
> @bdelacretaz @cziegeler any objections, otherwise I would merge.
@joerghoh since it is a false positive, you may want to add a "// NOSONAR" comment at the end of that line to suppress the warning.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857406442
Hi Carsten,
this problem is not restricted to the creation of paths only, but can happen with every JCR operation; the last instance I recall the repoinit execution failed when an exceptions was thrown during the registration of a mixin. Also the session.save() operation is centralized in the RepositoryInitializerFactory, changing it would require changes to all Operations.
Regarding logging: I will change it, so that only the exception of the last retry is logged; that means that if during the retry operation different RepositoryExceptions occurr the first ones will be swallowed and not logged.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856026033
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='55.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [55.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh merged pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh merged pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] kwin commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
kwin commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857446338
IMHO only https://docs.adobe.com/content/docs/en/spec/javax.jcr/javadocs/jcr-2.0/javax/jcr/InvalidItemStateException.html is transient. Can't we restrict retries to that exception class?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857426773
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] cziegeler commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
cziegeler commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857503970
I don't think that we need to make the retry configurable - if users need to fiddle around with this we should rather fix the implementation
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] cziegeler commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
cziegeler commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857429661
Registration of a mixin is a good example: I don't assume that this is a transient problem, or is it?
Thanks for solving the logging problem
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856016569
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D.png' alt='D' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857487885
> Moving the retries into these specific operations doesn't make much sense to me, as it would duplicate a lot of code with no benefit.
It is probably possible to design the code so that the various Operations can indicate if they need to be retried or not.
But as you say, the retrying implementation will do nothing in the vast majority of cases, and only slow down the actual failing cases. As repoinit is generally meant to be executed at startup only that's not really a problem.
I'll comment on the general discussions about retries at SLING-10418 as that's where the discussion started. I'm basically ok with adding the retry mechanism to everything as you suggested. Especially as you're only retrying on `InvalidItemStateException` now.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648073169
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +124,58 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
Review comment:
I think at least the maximum number of retries should be configurable. Together with logging the actual retry count as suggested below, this can help investigate problematic cases.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh edited a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh edited a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857440053
It is a transient problem, because mixins are also stored as part of the repo:
```
Caused by: javax.jcr.InvalidItemStateException: Failed to register node types.
at org.apache.jackrabbit.oak.api.CommitFailedException.asRepositoryException(CommitFailedException.java:238) [org.apache.jackrabbit.oak-api:1.39.0.R1889746]
...
Caused by: org.apache.jackrabbit.oak.api.CommitFailedException: OakState0001: Unresolved conflicts in /jcr:system/jcr:nodeTypes/dam:cfVariationNode
```
and can fail due to concurrency (as indicated by this OakState0001 exception). Same as path creation, user creation, ACL setup etc. Moving the retries into these specific operations doesn't make much sense to me, as it would duplicate a lot of code with no benefit.
There might be root causes where retry doesn't make sense because the operation will always fail (for example setting an ACL to a non-existing path). Avoiding to retry these is work because you need to review the different types of exceptions to understand if they indicate a transient or permanent failure, and if a retry makes sense.
Of course we can do that, but I consider the benefit very limited, because it would save ~6 seconds (retries after 1,2 and 3 seconds) until the repoinit operation finally fails. Also situations like this (incorrect repo init statements) should never the normal case, while concurrency issues are more likely to happen. The case mentioned above with perfectly valid rep init statements led to startup failures of a more than dozen cases.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856016569
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/D.png' alt='D' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-855973694
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [13 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='55.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [55.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857479263
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='53.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [53.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857460720
@kwin ah, good catch. Let me try to add this.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857530316
Looks good to me, just one nitpick on test coverage.
Building with `-P jacoco-report` shows a few methods in `RetryableOperationResult` which are not covered by tests: `shouldRetry`, `getFailureTrace` and `withJitterMsec`. If you can cover those as well that would be fantastic. And you can then merge, from my point of view. Thank you for this!
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857518046
Any comments left? Otherwise I would merge later today, so we can get this into the next jcr.repoinit release.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648086729
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +124,58 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
Review comment:
A hardcoded setting for the number of retries is what I'd like to avoid. What if someone thinks their operation is still failing with 3 retries but would work with 4 retries? Having the backoff duration hardcoded is probably ok, but I don't see a good reason for hardcoding the max retry count. Defaulting to 3 is certainly good.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856533401
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648106988
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +131,47 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws Exception {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBase(1000).withMaxRetries(3).build();
+ boolean successful = retry.apply(() -> {
+ try {
+ processor.apply(session, ops);
+ session.save();
+ return true;
+ } catch (RepositoryException e) {
+ log.error("(temporarily) failed to apply repoinit operations",e);
Review comment:
is now adjusted to log only the last exception.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] cziegeler commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
cziegeler commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r646651393
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +131,47 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws Exception {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBase(1000).withMaxRetries(3).build();
+ boolean successful = retry.apply(() -> {
+ try {
+ processor.apply(session, ops);
+ session.save();
+ return true;
+ } catch (RepositoryException e) {
+ log.error("(temporarily) failed to apply repoinit operations",e);
Review comment:
So in case of permanent failure we end up with the same exception being logged N (=3) times. We should avoid that
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r646639224
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RetryableOperation.java
##########
@@ -0,0 +1,120 @@
+/*
+ * 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.sling.jcr.repoinit.impl;
+
+import java.util.concurrent.TimeUnit;
+import java.util.function.Supplier;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * A simple implementation of retryable operations.
+ * Use the builder class to create an instance of it.
+ *
+ */
+public class RetryableOperation {
+
+ private static final Logger LOG = LoggerFactory.getLogger(RetryableOperation.class);
+
+ int backoffBase;
+ int maxRetries;
+ int jitter;
+
+ int retryCount = 0;
+
+ RetryableOperation(int backoff, int maxRetries, int jitter) {
+ this.backoffBase = backoff;
+ this.maxRetries = maxRetries;
+ this.jitter = jitter;
+ }
+ /**
+ * Execute the operation with the defined retry until it returns true or
+ * the retry aborts; in case the operation is retried, a log message is logged on INFO with the
+ * provided logMessage and the current number of retries
+ * @param operation
+ * @param logMessage the log message
+ * @return true if the supplier was eventually successful, false if it failed despite all retries
+ */
+ public boolean apply(Supplier<Boolean> operation, String logMessage) {
+
+ boolean successful = false;
+ successful = operation.get();
+ while (! successful && retryCount < maxRetries) {
+ retryCount++;
+ LOG.info("%s (retry %d/%d)", logMessage, retryCount, maxRetries);
+ delay(retryCount);
+ successful = operation.get();
+ }
+ return successful;
+ }
+
+ private void delay(int retryCount) {
+
+ int j = (int) (Math.random() * (jitter));
+ int delayInMilis = (backoffBase * retryCount) + j;
+ try {
+ TimeUnit.MILLISECONDS.sleep(delayInMilis);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
+ }
+
+
+
+ public static class Builder {
+
+ int exponentialBackoff = 1000; // default
+ int maxRetries = 3; // default
+ int jitter = 200;
+
+ /**
+ * The backoff time
+ * @param msec backoff time in miliseconds
+ * @return the builder
+ */
+ Builder withBackoffBase(int msec) {
Review comment:
Unless a method takes a Duration I like to express units in its name to avoid any ambiguities, so I would call this method `withBackoffBaseMsec`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-857426773
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [0 Code Smells](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='56.7%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [56.7% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] bdelacretaz commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
bdelacretaz commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648072532
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +124,58 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
+ RetryableOperation.RetryableOperationResult result = retry.apply(() -> {
+ try {
+ processor.apply(session, ops);
+ session.save();
+ return new RetryableOperation.RetryableOperationResult(true,false,null);
+ } catch (InvalidItemStateException ise) {
+ // a retry makes sense, because this exception might be caused by an concurrent operation
+ log.debug("(temporarily) failed to apply repoinit operations",ise);
+ try {
+ session.refresh(false); // discard all pending changes
+ } catch (RepositoryException e1) {
+ // ignore
+ }
+ return new RetryableOperation.RetryableOperationResult(false,true,ise);
+ } catch (RepositoryException re) {
+ // a permanent error, retry is not useful
+ try {
+ session.refresh(false); // discard all pending changes
+ } catch (RepositoryException e1) {
+ // ignore
+ }
+ return new RetryableOperation.RetryableOperationResult(false,false,re);
+ }
+ }, logMessage);
+ if (!result.isSuccessful()) {
Review comment:
If retries were needed, it would be good to log their count at the DEBUG level.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] joerghoh commented on a change in pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
joerghoh commented on a change in pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#discussion_r648076750
##########
File path: src/main/java/org/apache/sling/jcr/repoinit/impl/RepositoryInitializerFactory.java
##########
@@ -122,14 +124,58 @@ public void processRepository(final SlingRepository repo) throws Exception {
continue;
}
final List<Operation> ops = parser.parse(new StringReader(script));
- log.info("Executing {} repoinit operations", ops.size());
- processor.apply(s, ops);
- s.save();
+ String msg = String.format("Executing %s repoinit operations", ops.size());
+ log.info(msg);
+ applyOperations(s,ops,msg);
}
}
} finally {
s.logout();
}
}
}
+
+
+ /**
+ * Apply the operations within a session, support retries
+ * @param session the JCR session to use
+ * @param ops the list of operations
+ * @param logMessage the messages to print when retry
+ * @throws Exception if the application fails despite the retry
+ */
+ private void applyOperations(Session session, List<Operation> ops, String logMessage) throws RepositoryException {
+
+ RetryableOperation retry = new RetryableOperation.Builder().withBackoffBaseMsec(1000).withMaxRetries(3).build();
+ RetryableOperation.RetryableOperationResult result = retry.apply(() -> {
+ try {
+ processor.apply(session, ops);
+ session.save();
+ return new RetryableOperation.RetryableOperationResult(true,false,null);
+ } catch (InvalidItemStateException ise) {
+ // a retry makes sense, because this exception might be caused by an concurrent operation
+ log.debug("(temporarily) failed to apply repoinit operations",ise);
+ try {
+ session.refresh(false); // discard all pending changes
+ } catch (RepositoryException e1) {
+ // ignore
+ }
+ return new RetryableOperation.RetryableOperationResult(false,true,ise);
+ } catch (RepositoryException re) {
+ // a permanent error, retry is not useful
+ try {
+ session.refresh(false); // discard all pending changes
+ } catch (RepositoryException e1) {
+ // ignore
+ }
+ return new RetryableOperation.RetryableOperationResult(false,false,re);
+ }
+ }, logMessage);
+ if (!result.isSuccessful()) {
Review comment:
This is done as part of the RetryableOperation[1].
[1] https://github.com/apache/sling-org-apache-sling-jcr-repoinit/blob/feature/SLING-10418-retry/src/main/java/org/apache/sling/jcr/repoinit/impl/RetryableOperation.java#L60
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [sling-org-apache-sling-jcr-repoinit] sonarcloud[bot] removed a comment on pull request #18: SLING-10418 implement a retry for the application of the statements
Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] removed a comment on pull request #18:
URL: https://github.com/apache/sling-org-apache-sling-jcr-repoinit/pull/18#issuecomment-856026033
SonarCloud Quality Gate failed.
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug.png' alt='Bug' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG) [0 Bugs](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=BUG)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability.png' alt='Vulnerability' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=VULNERABILITY)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot.png' alt='Security Hotspot' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/E.png' alt='E' width='16' height='16' />](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT) [1 Security Hotspot](https://sonarcloud.io/project/security_hotspots?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=SECURITY_HOTSPOT)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell.png' alt='Code Smell' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A.png' alt='A' width='16' height='16' />](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL) [1 Code Smell](https://sonarcloud.io/project/issues?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&resolved=false&types=CODE_SMELL)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/50.png' alt='55.9%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list) [55.9% Coverage](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_coverage&view=list)
[<img src='https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/3.png' alt='0.0%' width='16' height='16' />](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list) [0.0% Duplication](https://sonarcloud.io/component_measures?id=apache_sling-org-apache-sling-jcr-repoinit&pullRequest=18&metric=new_duplicated_lines_density&view=list)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org