[
https://issues.apache.org/jira/browse/GOBBLIN-2016?focusedWorklogId=924895&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-924895
]
ASF GitHub Bot logged work on GOBBLIN-2016:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 08/Jul/24 20:52
Start Date: 08/Jul/24 20:52
Worklog Time Spent: 10m
Work Description: phet commented on code in PR #3995:
URL: https://github.com/apache/gobblin/pull/3995#discussion_r1669282212
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagActionReminderScheduler.java:
##########
@@ -128,23 +128,27 @@ public void execute(JobExecutionContext context) {
}
/**
- * Creates a key for the reminder job by concatenating all dagAction fields
+ * Creates a key for the reminder job by concatenating all dagAction fields
and the eventTime of the dagAction to
+ * allow reminders for actions associated with multiple flow executions
within a deadline period (e.g. another
+ * flow execution may occur before a flow finish or job start deadline
expires)
*/
- public static String createDagActionReminderKey(DagActionStore.DagAction
dagAction) {
- return String.format("%s.%s.%s.%s.%s", dagAction.getFlowGroup(),
dagAction.getFlowName(),
- dagAction.getFlowExecutionId(), dagAction.getJobName(),
dagAction.getDagActionType());
+ public static String createDagActionReminderKey(DagActionStore.LeaseParams
leaseParams) {
+ DagActionStore.DagAction dagAction = leaseParams.getDagAction();
+ return String.format("%s.%s.%s.%s.%s.%s", dagAction.getFlowGroup(),
dagAction.getFlowName(),
Review Comment:
this seems clearer:
```
String.join(".",
dagAction.getFlowGroup(),
dagAction.getFlowName(),
dagAction.getFlowExecutionId().toString(),
dagAction.getJobName(),
dagAction.getDagActionType(),
leaseParams.getEventTimeMillis().toString());
```
##########
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagActionReminderScheduler.java:
##########
@@ -128,23 +128,27 @@ public void execute(JobExecutionContext context) {
}
/**
- * Creates a key for the reminder job by concatenating all dagAction fields
+ * Creates a key for the reminder job by concatenating all dagAction fields
and the eventTime of the dagAction to
+ * allow reminders for actions associated with multiple flow executions
within a deadline period (e.g. another
+ * flow execution may occur before a flow finish or job start deadline
expires)
Review Comment:
this doesn't really capture the nuance of why it's insufficient to
incorporate only the flowExecId into the key. explain why an event timestamp
is needed in addition to that flowExecId.
doesn't it have to do w/ event consolidation?
Issue Time Tracking
-------------------
Worklog Id: (was: 924895)
Time Spent: 0.5h (was: 20m)
> retry retry-able exceptions in DagProcs
> ---------------------------------------
>
> Key: GOBBLIN-2016
> URL: https://issues.apache.org/jira/browse/GOBBLIN-2016
> Project: Apache Gobblin
> Issue Type: Task
> Reporter: Arjun Singh Bora
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
