gnodet-bot commented on code in PR #29466:
URL: https://github.com/apache/camel/pull/29466#discussion_r4512104333
##########
components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java:
##########
@@ -320,9 +334,14 @@ public int changeRule(String id, String rule) {
}
checkRoute(definition);
long now = TimeSource.currentTimeMillis();
+ checkDeadline(now);
ChaosRule changed = new ChaosRule(definition, id,
rules.get(index).version + 1, seed, now, maxDuration);
changed.recentFaults = recentFaults;
changed.clockStarted = camelContext.isStarted();
Review Comment:
🐛 **Bug — `IndexOutOfBoundsException` in `changeRule()`**
`checkDeadline(now)` on this line can call `finish()` → `clearRules()`,
which empties `rules`. The very next line (`rules.get(index)`) then throws
`IndexOutOfBoundsException` because `index` is now stale.
The simplest fix is to guard continued execution after `checkDeadline`:
```suggestion
checkDeadline(now);
if (exploration == null || !exploration.isRunning()) {
return -1; // exploration ended, rule no longer valid
}
```
Or alternatively, move `checkDeadline` after `rules.get(index)` has been
used (before `rules.set`).
##########
components/camel-chaos/src/main/java/org/apache/camel/component/chaos/ChaosManager.java:
##########
@@ -600,9 +629,105 @@ public ChaosReport report() {
List<ChaosRule> all = new ArrayList<>(current);
all.addAll(removed);
List<ChaosAdvisor.RouteTried> tried = ChaosAdvisor.tried(all,
suggestedKinds(now), now);
- return new ChaosReport(
+ ChaosReport report = new ChaosReport(
seed, seedFrom, now, current, removed, counts, unmet, lost,
pending,
lostAtDlq, recentFaults.latestFirst(),
ChaosTargets.reach(camelContext), tried);
+ if (explored != null) {
+ synchronized (lock) {
+ report.exploration(explored.toJson(now), explored.text(now));
+ }
+ }
+ return report;
+ }
+
+ /**
+ * Starts an exploration (CAMEL-26773): an agent tries chaos rules for the
time it was given. While it runs, a rule
+ * ends at the deadline at the latest, and at the deadline the rules are
removed and the exploration is finished.
+ * The report has its journal: the baseline (the counts per route now) and
the rules added while it runs.
+ *
+ * @param budget how long, such as 10m (10 minutes when
empty, at most 2 hours)
+ * @param focus what the agent explores, such as a
route (optional)
+ * @return the exploration, as in the report
+ * @throws IllegalArgumentException when the budget is not a time, or too
short or too long
+ * @throws IllegalStateException when an exploration is running
+ */
+ public Map<String, Object> startExploration(String budget, String focus) {
+ long millis;
+ try {
+ millis = budget == null || budget.isBlank()
+ ? ChaosExploration.DEFAULT_BUDGET :
TimeUtils.toMilliSeconds(budget.strip());
+ } catch (RuntimeException e) {
+ throw new IllegalArgumentException("The budget of the exploration
is not a time: " + budget);
+ }
+ if (millis < ChaosExploration.MIN_BUDGET || millis >
ChaosExploration.MAX_BUDGET) {
+ throw new IllegalArgumentException(
+ "The budget of the exploration must be between 10s and 2h,
was: " + budget);
+ }
+ synchronized (lock) {
+ long now = TimeSource.currentTimeMillis();
+ checkDeadline(now);
+ if (exploration != null && exploration.isRunning()) {
+ throw new IllegalStateException(
+ "An exploration is running, for another " +
ChaosRuleParser.printTime(exploration.deadline - now)
+ + ": finish it first");
+ }
+ exploration = new ChaosExploration(now, millis, focus);
+ update();
+ LOG.info("Chaos exploration started: {}{}",
ChaosRuleParser.printTime(millis),
+ exploration.focus != null ? " (focus: " +
exploration.focus + ")" : "");
+ return exploration.toJson(now);
+ }
+ }
+
+ /**
+ * Finishes the running exploration before its deadline: its rules are
removed, and its journal stays in the report
+ * until the next one starts or the report is reset.
+ *
+ * @return the exploration, as in the report
+ * @throws IllegalStateException when no exploration is running
+ */
+ public Map<String, Object> finishExploration() {
+ synchronized (lock) {
+ long now = TimeSource.currentTimeMillis();
+ checkDeadline(now);
+ if (exploration == null || !exploration.isRunning()) {
+ throw new IllegalStateException("No exploration is running");
+ }
+ finish(now, ChaosExploration.Finish.FINISHED);
+ return exploration.toJson(now);
+ }
+ }
+
+ // finishes the running exploration whose time is up; with the lock
+ private void checkDeadline(long now) {
+ if (exploration != null && exploration.isRunning() && now >=
exploration.deadline) {
+ finish(now, ChaosExploration.Finish.DEADLINE);
+ }
+ }
+
+ private void finish(long now, ChaosExploration.Finish by) {
+ int removedRules = clearRules();
+ exploration.finished(now, by, removedRules);
+ LOG.info("Chaos exploration {}: {} rules tried, {} removed",
+ by == ChaosExploration.Finish.DEADLINE ? "finished at its
deadline" : "finished",
Review Comment:
🐛 **Bug — `counting` left enabled after exploration ends**
`clearRules()` calls `update()`, which checks `exploration.isRunning()` — at
this point it's still `true`, so `counting` is set to `true`. Then
`exploration.finished()` marks the exploration done, but no subsequent
`update()` call resets `counting` to `false`. After the exploration ends,
exchange counting remains enabled indefinitely until the next rule is
added/removed.
Consider resetting `counting` explicitly after `exploration.finished()`, or
calling `update()` once more after it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
