This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit a5403f5d1a96a4377e889802bb3524a0973d3a25 Author: Alex Heneveld <[email protected]> AuthorDate: Sun Jun 11 10:51:30 2023 +0100 remove old vars and a couple fixes to ensure the right data, when injected initially or from errors --- .../core/workflow/WorkflowExecutionContext.java | 24 +++++++--------------- .../workflow/WorkflowExpressionResolution.java | 2 +- .../core/workflow/steps/CustomWorkflowStep.java | 2 +- .../brooklyn/core/workflow/WorkflowSizeTest.java | 3 ++- 4 files changed, 11 insertions(+), 20 deletions(-) diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java index ec95ed99d6..26fc66df27 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExecutionContext.java @@ -220,7 +220,6 @@ public class WorkflowExecutionContext { WorkflowStepInstanceExecutionContext context; /** is step replayable */ Boolean replayableFromHere; - transient Map<String,Object> workflowScratchOld; /** scratch vars as at start of last invocation of set _if_ they could not be derived from updates */ Map<String,Object> workflowScratch; /** updates to scratch vars made by the last run of this step */ @@ -234,8 +233,7 @@ public class WorkflowExecutionContext { String nextTaskId; } - // note: when persisted, this may be omitted and restored from the oldStepInfo map in the Converter - transient Map<String,Object> workflowScratchVariablesOld = MutableMap.of(); + // when persisted, this is omitted and restored from the oldStepInfo map on read transient Map<String,Object> workflowScratchVariables; transient Map<String,Object> workflowScratchVariablesUpdatedThisStep; @@ -409,15 +407,12 @@ public class WorkflowExecutionContext { Pair<Map<String, Object>, Set<Integer>> prev = getStepWorkflowScratchAndBacktrackedSteps(null); workflowScratchVariables = prev.getLeft(); } - checkEqual(workflowScratchVariablesOld, workflowScratchVariables); return MutableMap.copyOf(workflowScratchVariables).asUnmodifiable(); } public Object updateWorkflowScratchVariable(String s, Object v) { if (workflowScratchVariables ==null) getWorkflowScratchVariables(); - workflowScratchVariables.put(s, v); - Object old = workflowScratchVariablesOld.put(s, v); - if (v==null) workflowScratchVariablesOld.remove(s); + Object old = workflowScratchVariables.put(s, v); if (v==null) workflowScratchVariables.remove(s); if (workflowScratchVariablesUpdatedThisStep==null) workflowScratchVariablesUpdatedThisStep = MutableMap.of(); workflowScratchVariablesUpdatedThisStep.put(s, v); @@ -428,7 +423,6 @@ public class WorkflowExecutionContext { if (newValues!=null && !newValues.isEmpty()) { if (workflowScratchVariables ==null) getWorkflowScratchVariables(); workflowScratchVariables.putAll(newValues); - workflowScratchVariablesOld.putAll(newValues); if (workflowScratchVariablesUpdatedThisStep==null) workflowScratchVariablesUpdatedThisStep = MutableMap.of(); workflowScratchVariablesUpdatedThisStep.putAll(newValues); } @@ -1282,7 +1276,6 @@ public class WorkflowExecutionContext { } // and ensure not null if (workflowScratchVariables == null) workflowScratchVariables = MutableMap.of(); - workflowScratchVariablesOld = workflowScratchVariables; } private void initializeFromContinuationInstructions(Integer replayFromStep) { @@ -1433,13 +1426,6 @@ public class WorkflowExecutionContext { if (old == null) old = new OldStepRecord(); old.countStarted++; - // no longer necessary because can recompute, and any specials needed at start will have been applied in one of the updateOldNextStepOnThisStepStarting - if (!workflowScratchVariablesOld.isEmpty()) { - old.workflowScratchOld = MutableMap.copyOf(workflowScratchVariablesOld); - } else { - old.workflowScratchOld = null; - } - old.workflowScratchUpdates = null; old.previous = MutableSet.<Integer>of(previousStepIndex == null ? STEP_INDEX_FOR_START : previousStepIndex).putAll(old.previous); @@ -1450,6 +1436,10 @@ public class WorkflowExecutionContext { WorkflowReplayUtils.updateReplayableFromStep(WorkflowExecutionContext.this, step); oldStepInfo.compute(previousStepIndex==null ? STEP_INDEX_FOR_START : previousStepIndex, (index, old) -> { if (old==null) old = new OldStepRecord(); + if (previousStepIndex==null && workflowScratchVariables!=null && !workflowScratchVariables.isEmpty()) { + // if workflow scratch vars were initialized prior to run, we nee to save those + old.workflowScratch = MutableMap.copyOf(workflowScratchVariables); + } old.next = MutableSet.<Integer>of(currentStepIndex).putAll(old.next); old.nextTaskId = t.getId(); return old; @@ -1470,7 +1460,7 @@ public class WorkflowExecutionContext { } if (currentStepInstance.output != null) { Pair<Object, Set<Integer>> prev = getStepOutputAndBacktrackedSteps(null); - if (prev != null && Objects.equals(prev.getLeft(), currentStepInstance.output)) { + if (prev != null && Objects.equals(prev.getLeft(), currentStepInstance.output) && lastErrorHandlerOutput==null) { // optimization, clear the value here if we can simply take it from the previous step currentStepInstance.output = null; } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java index b13ad09334..b3e6c0589f 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/WorkflowExpressionResolution.java @@ -116,7 +116,7 @@ public class WorkflowExpressionResolution { if (context.getOutput()!=null) return TemplateProcessor.wrapAsTemplateModel(context.getOutput()); if (context.currentStepInstance!=null && context.currentStepInstance.getOutput() !=null) return TemplateProcessor.wrapAsTemplateModel(context.currentStepInstance.getOutput()); Object previousStepOutput = context.getPreviousStepOutput(); - if (previousStepOutput !=null) return TemplateProcessor.wrapAsTemplateModel(previousStepOutput); + if (previousStepOutput!=null) return TemplateProcessor.wrapAsTemplateModel(previousStepOutput); return ifNoMatches(); } diff --git a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java index 1fd5ca6202..1a4d5a9200 100644 --- a/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java +++ b/core/src/main/java/org/apache/brooklyn/core/workflow/steps/CustomWorkflowStep.java @@ -542,7 +542,7 @@ public class CustomWorkflowStep extends WorkflowStepDefinition implements Workfl String tivn = context.resolve(WorkflowExpressionResolution.WorkflowExpressionStage.STEP_INPUT, target_index_var_name, String.class); - if (targetIndexOrNull!=null) nestedWorkflowContext.updateWorkflowScratchVariable(tivn==null ? TARGET_INDEX_VAR_NAME_DEFAULT : tivn, targetIndexOrNull); + if (targetIndexOrNull!=null) nestedWorkflowContext.updateWorkflowScratchVariable(tivn == null ? TARGET_INDEX_VAR_NAME_DEFAULT : tivn, targetIndexOrNull); initializeSubWorkflowForTarget(context, target, nestedWorkflowContext); return nestedWorkflowContext; diff --git a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSizeTest.java b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSizeTest.java index 4edcb936cc..ec81b25d8c 100644 --- a/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSizeTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/workflow/WorkflowSizeTest.java @@ -96,9 +96,10 @@ public class WorkflowSizeTest extends BrooklynMgmtUnitTestSupport { Asserts.assertThat(sizes.values().stream().reduce(0, (v0,v1)->v0+v1), result -> result < 20*1000); // 100k payload now -> bumps sensor (json) size from 5k to 3MB (before any optimization) + // [xml persistence is less of an issue because it will use a shared reference] // removing output which is identical to the previous gives minor savings (in this test): 3380416 -> 3176074 // removing scratch at workflow which matches a step reduces further: -> 2869522 - // [xml persistence is less of an issue because it will use a shared reference] + // switching to model where scratch is produced incrementally reduces much more -> 1847848 for (int i=0; i<1000; i++) { for (int j=0; j<10; j++) sampleData += "0123456789"; sampleData += "\n";
