xintongsong commented on code in PR #181:
URL: https://github.com/apache/flink-agents/pull/181#discussion_r2371573191
##########
runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java:
##########
@@ -190,9 +190,17 @@ public void open() throws Exception {
&& KAFKA.getType()
.equalsIgnoreCase(agentPlan.getConfig().get(ACTION_STATE_STORE_BACKEND))) {
LOG.info("Using Kafka as backend of action state store.");
- actionStateStore = new KafkaActionStateStore();
+ actionStateStore = new
KafkaActionStateStore(agentPlan.getConfig());
}
+ // init recovery marker state for recovery marker persistence
+ recoveryMarkerOpState =
+ getOperatorStateBackend()
+ .getUnionListState(
+ new ListStateDescriptor<>(
+ RECOVERY_MARKER_STATE_NAME,
+ TypeInformation.of(Object.class)));
Review Comment:
Minor: We might not need this if `actionStateStore == null`
##########
runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java:
##########
@@ -350,13 +358,12 @@ private void processActionTaskForKey(Object key) throws
Exception {
long sequenceNumber = sequenceNumberKState.value();
boolean isFinished;
List<Event> outputEvents;
- Optional<ActionTask> generatedActionTaskOpt;
+ Optional<ActionTask> generatedActionTaskOpt = Optional.empty();
ActionState actionState =
maybeGetActionState(key, sequenceNumber, actionTask.action,
actionTask.event);
- if (actionState != null &&
actionState.getGeneratedActionTask().isEmpty()) {
+ if (actionState != null) {
Review Comment:
I think we need to either check that action task is finished before calling
`maybePersistTaskResult()`, or skip the persisting for unfinished actions
inside `maybePersistTaskResult()`.
##########
runtime/src/main/java/org/apache/flink/agents/runtime/operator/ActionExecutionOperator.java:
##########
@@ -350,13 +358,12 @@ private void processActionTaskForKey(Object key) throws
Exception {
long sequenceNumber = sequenceNumberKState.value();
boolean isFinished;
List<Event> outputEvents;
- Optional<ActionTask> generatedActionTaskOpt;
+ Optional<ActionTask> generatedActionTaskOpt = Optional.empty();
ActionState actionState =
maybeGetActionState(key, sequenceNumber, actionTask.action,
actionTask.event);
- if (actionState != null &&
actionState.getGeneratedActionTask().isEmpty()) {
+ if (actionState != null) {
Review Comment:
I assume this means we expect an incompleted action should not have a action
state in the store? How is this guaranteed? Looks like we always call
`maybePersistTaskResult` after `actionTask.invoke()`.
--
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]
