Skip to content

Commit 97b11fa

Browse files
committed
feat: Implement human-in-the-loop support for while loop (#166)
Signed-off-by: Jagan Nalla <jagannalla1@gmail.com>
1 parent 37eb6ae commit 97b11fa

12 files changed

Lines changed: 184 additions & 77 deletions

File tree

maestro-engine/src/main/java/com/netflix/maestro/engine/dao/MaestroStepInstanceDao.java

Lines changed: 33 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ public class MaestroStepInstanceDao extends AbstractDatabaseDao {
9191

9292
private static final String UPDATE_STEP_INSTANCE_QUERY =
9393
"UPDATE maestro_step_instance SET (runtime_state,dependencies,outputs,artifacts,timeline,generation_id) = (?::jsonb,?::jsonb,?::jsonb,?::jsonb,?,?) "
94-
+ WHERE_CONDITION_BY_IDS + " AND generation_id <= ?";
94+
+ WHERE_CONDITION_BY_IDS
95+
+ " AND generation_id <= ?";
9596

9697
private static final String SELECT_STEP_FIELDS = "SELECT %s FROM maestro_step_instance ";
9798

@@ -221,7 +222,10 @@ public void insertOrUpsertStepInstance(
221222
}
222223

223224
public void insertOrUpsertStepInstance(
224-
StepInstance instance, boolean inserted, @Nullable MaestroJobEvent jobEvent, long flowGeneration) {
225+
StepInstance instance,
226+
boolean inserted,
227+
@Nullable MaestroJobEvent jobEvent,
228+
long flowGeneration) {
225229
final StepRuntimeState runtimeState = instance.getRuntimeState();
226230
final SignalDependencies dependencies = instance.getSignalDependencies();
227231
final SignalOutputs outputs = instance.getSignalOutputs();
@@ -249,33 +253,33 @@ public void insertOrUpsertStepInstance(
249253
() ->
250254
withRetryableTransaction(
251255
conn -> {
252-
try (PreparedStatement stmt =
253-
conn.prepareStatement(
254-
inserted
255-
? UPSERT_STEP_INSTANCE_QUERY
256-
: CREATE_STEP_INSTANCE_QUERY)) {
257-
int idx = 0;
258-
stmt.setString(++idx, instance.getWorkflowId());
259-
stmt.setLong(++idx, instance.getWorkflowInstanceId());
260-
stmt.setLong(++idx, instance.getWorkflowRunId());
261-
stmt.setString(++idx, instance.getStepId());
262-
stmt.setLong(++idx, instance.getStepAttemptId());
263-
stmt.setString(++idx, instance.getWorkflowUuid());
264-
stmt.setString(++idx, instance.getStepUuid());
265-
stmt.setString(++idx, instance.getCorrelationId());
266-
stmt.setString(++idx, stepInstanceStr);
267-
stmt.setString(++idx, runtimeStateStr);
268-
stmt.setString(++idx, stepDependenciesSummariesStr);
269-
stmt.setString(++idx, outputsStr);
270-
stmt.setString(++idx, artifactsStr);
271-
stmt.setArray(++idx, conn.createArrayOf(ARRAY_TYPE_NAME, timelineArray));
272-
stmt.setLong(++idx, flowGeneration);
273-
int res = stmt.executeUpdate();
274-
if (res == SUCCESS_WRITE_SIZE && jobEvent != null) {
275-
return queueSystem.enqueue(conn, jobEvent);
276-
}
277-
return null;
278-
}
256+
try (PreparedStatement stmt =
257+
conn.prepareStatement(
258+
inserted
259+
? UPSERT_STEP_INSTANCE_QUERY
260+
: CREATE_STEP_INSTANCE_QUERY)) {
261+
int idx = 0;
262+
stmt.setString(++idx, instance.getWorkflowId());
263+
stmt.setLong(++idx, instance.getWorkflowInstanceId());
264+
stmt.setLong(++idx, instance.getWorkflowRunId());
265+
stmt.setString(++idx, instance.getStepId());
266+
stmt.setLong(++idx, instance.getStepAttemptId());
267+
stmt.setString(++idx, instance.getWorkflowUuid());
268+
stmt.setString(++idx, instance.getStepUuid());
269+
stmt.setString(++idx, instance.getCorrelationId());
270+
stmt.setString(++idx, stepInstanceStr);
271+
stmt.setString(++idx, runtimeStateStr);
272+
stmt.setString(++idx, stepDependenciesSummariesStr);
273+
stmt.setString(++idx, outputsStr);
274+
stmt.setString(++idx, artifactsStr);
275+
stmt.setArray(++idx, conn.createArrayOf(ARRAY_TYPE_NAME, timelineArray));
276+
stmt.setLong(++idx, flowGeneration);
277+
int res = stmt.executeUpdate();
278+
if (res == SUCCESS_WRITE_SIZE && jobEvent != null) {
279+
return queueSystem.enqueue(conn, jobEvent);
280+
}
281+
return null;
282+
}
279283
}),
280284
"insertOrUpsertStepInstance",
281285
"Failed to insert or upsert step instance {}[{}]",

maestro-engine/src/main/java/com/netflix/maestro/engine/execution/StepLocalMemory.java

Lines changed: 29 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
import java.util.concurrent.ConcurrentHashMap;
1818

1919
/**
20-
* Transient in-memory storage for step runtimes to store step-instance-scoped states.
21-
* States are NOT persisted and will be lost on JVM reboot.
20+
* Transient in-memory storage for step runtimes to store step-instance-scoped states. States are
21+
* NOT persisted and will be lost on JVM reboot.
2222
*/
2323
public final class StepLocalMemory {
2424
private static final Map<String, Map<String, Object>> MEMORY_MAP = new ConcurrentHashMap<>();
@@ -28,39 +28,40 @@ public final class StepLocalMemory {
2828
private StepLocalMemory() {}
2929

3030
/**
31-
* Get or create a transient memory map for the specified step instance.
32-
* Scoped to the step instance run.
31+
* Get or create a transient memory map for the specified step instance. Scoped to the step
32+
* instance run.
3333
*/
3434
public static Map<String, Object> getOrCreate(String stepInstanceUuid) {
3535
if (stepInstanceUuid == null) {
3636
return new ConcurrentHashMap<>();
3737
}
38-
return MEMORY_MAP.computeIfAbsent(stepInstanceUuid, k -> new ConcurrentHashMap<String, Object>() {
39-
@Override
40-
public Object put(String key, Object value) {
41-
Object old = super.put(key, value);
42-
checkSize(this);
43-
return old;
44-
}
38+
return MEMORY_MAP.computeIfAbsent(
39+
stepInstanceUuid,
40+
k ->
41+
new ConcurrentHashMap<String, Object>() {
42+
@Override
43+
public Object put(String key, Object value) {
44+
Object old = super.put(key, value);
45+
checkSize(this);
46+
return old;
47+
}
4548

46-
@Override
47-
public void putAll(Map<? extends String, ?> m) {
48-
super.putAll(m);
49-
checkSize(this);
50-
}
49+
@Override
50+
public void putAll(Map<? extends String, ?> m) {
51+
super.putAll(m);
52+
checkSize(this);
53+
}
5154

52-
@Override
53-
public Object putIfAbsent(String key, Object value) {
54-
Object old = super.putIfAbsent(key, value);
55-
checkSize(this);
56-
return old;
57-
}
58-
});
55+
@Override
56+
public Object putIfAbsent(String key, Object value) {
57+
Object old = super.putIfAbsent(key, value);
58+
checkSize(this);
59+
return old;
60+
}
61+
});
5962
}
6063

61-
/**
62-
* Remove the step instance memory map.
63-
*/
64+
/** Remove the step instance memory map. */
6465
public static void remove(String stepInstanceUuid) {
6566
if (stepInstanceUuid != null) {
6667
MEMORY_MAP.remove(stepInstanceUuid);
@@ -72,7 +73,8 @@ private static void checkSize(Map<String, Object> map) {
7273
byte[] bytes = OBJECT_MAPPER.writeValueAsBytes(map);
7374
if (bytes.length > SIZE_LIMIT_BYTES) {
7475
throw new IllegalArgumentException(
75-
String.format("Step local memory size limit exceeded: %d bytes (limit: %d bytes)",
76+
String.format(
77+
"Step local memory size limit exceeded: %d bytes (limit: %d bytes)",
7678
bytes.length, SIZE_LIMIT_BYTES));
7779
}
7880
} catch (IllegalArgumentException e) {

maestro-engine/src/main/java/com/netflix/maestro/engine/execution/StepRuntimeManager.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,9 @@ public boolean execute(
155155
switch (result.state()) {
156156
case CONTINUE:
157157
return true;
158+
case PAUSED:
159+
runtimeSummary.markPaused(tracingManager);
160+
return true;
158161
case DONE:
159162
runtimeSummary.markFinishing(tracingManager);
160163
return result.shouldPersist();

maestro-engine/src/main/java/com/netflix/maestro/engine/execution/StepRuntimeSummary.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -406,9 +406,7 @@ public String getIdentity() {
406406
return String.format("[%s][%s][%s]", stepId, stepAttemptId, stepInstanceUuid);
407407
}
408408

409-
/**
410-
* Get the transient local memory map for this step instance.
411-
*/
409+
/** Get the transient local memory map for this step instance. */
412410
@JsonIgnore
413411
public Map<String, Object> getLocalMemory() {
414412
return StepLocalMemory.getOrCreate(stepInstanceUuid);

maestro-engine/src/main/java/com/netflix/maestro/engine/execution/StepSyncManager.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,10 @@ public Optional<Details> sync(
7070
case INSERT:
7171
case UPSERT:
7272
instanceDao.insertOrUpsertStepInstance(
73-
instance, stepSummary.getDbOperation() == DbOperation.UPSERT, jobEvent, flowGeneration);
73+
instance,
74+
stepSummary.getDbOperation() == DbOperation.UPSERT,
75+
jobEvent,
76+
flowGeneration);
7477
break;
7578
case UPDATE:
7679
instanceDao.updateStepInstance(workflowSummary, stepSummary, jobEvent, flowGeneration);

maestro-engine/src/main/java/com/netflix/maestro/engine/steps/StepRuntime.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,17 @@ enum State {
9393
/** the step becomes STOPPED terminate state. */
9494
STOPPED,
9595
/** the step becomes TIMED_OUT terminate state. */
96-
TIMED_OUT;
96+
TIMED_OUT,
97+
/** the step is paused. */
98+
PAUSED;
9799

98100
public boolean isFailed() {
99101
// Note that TIMED_OUT is currently considered as failed.
100-
return this != CONTINUE && this != DONE && this != STOPPED;
102+
return this != CONTINUE && this != DONE && this != STOPPED && this != PAUSED;
101103
}
102104

103105
public boolean isTerminal() {
104-
return this != CONTINUE;
106+
return this != CONTINUE && this != PAUSED;
105107
}
106108
}
107109

maestro-engine/src/main/java/com/netflix/maestro/engine/steps/WhileStepRuntime.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package com.netflix.maestro.engine.steps;
1414

1515
import com.netflix.maestro.engine.concurrency.InstanceStepConcurrencyHandler;
16+
import com.netflix.maestro.engine.dao.MaestroStepBreakpointDao;
1617
import com.netflix.maestro.engine.dao.MaestroStepInstanceDao;
1718
import com.netflix.maestro.engine.dao.MaestroWorkflowInstanceDao;
1819
import com.netflix.maestro.engine.eval.ParamEvaluator;
@@ -80,6 +81,7 @@ public class WhileStepRuntime implements StepRuntime {
8081
private final WorkflowActionHandler actionHandler;
8182
private final MaestroWorkflowInstanceDao instanceDao;
8283
private final MaestroStepInstanceDao stepInstanceDao;
84+
private final MaestroStepBreakpointDao stepBreakpointDao;
8385
private final MaestroQueueSystem queueSystem;
8486
private final InstanceStepConcurrencyHandler instanceStepConcurrencyHandler;
8587
private final ParamEvaluator paramEvaluator;
@@ -89,6 +91,14 @@ public class WhileStepRuntime implements StepRuntime {
8991
public Result start(
9092
WorkflowSummary workflowSummary, Step step, StepRuntimeSummary runtimeSummary) {
9193
try {
94+
if (runtimeSummary.getArtifacts().containsKey(Artifact.Type.WHILE.key())) {
95+
return new Result(
96+
State.DONE,
97+
Collections.singletonMap(
98+
Artifact.Type.WHILE.key(),
99+
runtimeSummary.getArtifacts().get(Artifact.Type.WHILE.key())),
100+
Collections.emptyList());
101+
}
92102
Artifact artifact = createArtifact(workflowSummary, runtimeSummary);
93103
return new Result(
94104
State.DONE,
@@ -233,6 +243,19 @@ public Result execute(
233243
trackWhileIteration(workflowSummary, runtimeSummary, (WhileStep) step, artifact);
234244

235245
if (result == null) {
246+
if (stepBreakpointDao.createPausedStepAttemptIfNeeded(
247+
workflowSummary.getWorkflowId(),
248+
workflowSummary.getWorkflowVersionId(),
249+
workflowSummary.getWorkflowInstanceId(),
250+
workflowSummary.getWorkflowRunId(),
251+
runtimeSummary.getStepId(),
252+
runtimeSummary.getStepAttemptId())) {
253+
return new Result(
254+
State.PAUSED,
255+
Collections.singletonMap(artifact.getType().key(), artifact),
256+
Collections.singletonList(
257+
TimelineLogEvent.info("While loop paused between iterations due to breakpoint")));
258+
}
236259
return runWhileIteration(workflowSummary, (WhileStep) step, runtimeSummary, artifact);
237260
}
238261
return result;

maestro-engine/src/main/java/com/netflix/maestro/engine/tasks/MaestroTask.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1001,7 +1001,8 @@ private void syncPendingUpdates(
10011001
boolean thrown,
10021002
long flowGeneration) {
10031003
StepInstance stepInstance = createStepInstance(workflowSummary, runtimeSummary);
1004-
Optional<Details> result = stepSyncManager.sync(stepInstance, workflowSummary, runtimeSummary, flowGeneration);
1004+
Optional<Details> result =
1005+
stepSyncManager.sync(stepInstance, workflowSummary, runtimeSummary, flowGeneration);
10051006
if (result.isPresent()) {
10061007
runtimeSummary.addTimeline(
10071008
TimelineLogEvent.warn("Failed to sync due to error: " + result.get()));

maestro-engine/src/test/java/com/netflix/maestro/engine/dao/MaestroStepInstanceDaoTest.java

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -694,7 +694,8 @@ public void testStepInstanceGenerationId() throws Exception {
694694
siGen2.setStepId("job1");
695695
try {
696696
stepDao.getStepInstance(TEST_WORKFLOW_ID, 1, 2, "job1", "1");
697-
} catch (MaestroNotFoundException ignored) {}
697+
} catch (MaestroNotFoundException ignored) {
698+
}
698699

699700
stepDao.insertOrUpsertStepInstance(siGen2, false, null, 2L);
700701

@@ -703,16 +704,17 @@ public void testStepInstanceGenerationId() throws Exception {
703704
workflowSummary.setWorkflowInstanceId(1);
704705
workflowSummary.setWorkflowRunId(2);
705706

706-
StepRuntimeSummary summary = StepRuntimeSummary.builder()
707-
.stepId("job1")
708-
.stepAttemptId(1)
709-
.stepInstanceId(1)
710-
.runtimeState(siGen2.getRuntimeState())
711-
.artifacts(siGen2.getArtifacts())
712-
.signalDependencies(siGen2.getSignalDependencies())
713-
.signalOutputs(siGen2.getSignalOutputs())
714-
.timeline(siGen2.getTimeline())
715-
.build();
707+
StepRuntimeSummary summary =
708+
StepRuntimeSummary.builder()
709+
.stepId("job1")
710+
.stepAttemptId(1)
711+
.stepInstanceId(1)
712+
.runtimeState(siGen2.getRuntimeState())
713+
.artifacts(siGen2.getArtifacts())
714+
.signalDependencies(siGen2.getSignalDependencies())
715+
.signalOutputs(siGen2.getSignalOutputs())
716+
.timeline(siGen2.getTimeline())
717+
.build();
716718

717719
siGen2.getRuntimeState().setStatus(StepInstance.Status.FATALLY_FAILED);
718720
summary.getRuntimeState().setStatus(StepInstance.Status.FATALLY_FAILED);

maestro-engine/src/test/java/com/netflix/maestro/engine/execution/StepSyncManagerTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ public void testUpdatePendingRecords() {
145145
assertFalse(details.isPresent());
146146
var eventCaptor = ArgumentCaptor.forClass(MaestroJobEvent.class);
147147
verify(instanceDao, times(1))
148-
.updateStepInstance(eq(workflowSummary), eq(stepRuntimeSummary), eventCaptor.capture(), eq(1L));
148+
.updateStepInstance(
149+
eq(workflowSummary), eq(stepRuntimeSummary), eventCaptor.capture(), eq(1L));
149150
assertEquals(NotificationJobEvent.class, eventCaptor.getValue().getClass());
150151
}
151152

0 commit comments

Comments
 (0)