jack-moseley commented on a change in pull request #3220:
URL: https://github.com/apache/gobblin/pull/3220#discussion_r572406510
##########
File path:
gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagManager.java
##########
@@ -1176,6 +1192,39 @@ private synchronized void cleanUpDag(String dagId) {
}
}
+ /**
+ * Thread that runs retention on failed dags based on their original start
time (from flow execution ID).
+ */
+ public static class FailedDagRetentionThread implements Runnable {
+ private final DagStateStore failedDagStateStore;
+ private final Map<String, Dag<JobExecutionPlan>> failedDags;
+ private final long failedDagRetentionTime;
+
+ FailedDagRetentionThread(DagStateStore failedDagStateStore, Map<String,
Dag<JobExecutionPlan>> failedDags, long failedDagRetentionTime) {
+ this.failedDagStateStore = failedDagStateStore;
+ this.failedDags = failedDags;
+ this.failedDagRetentionTime = failedDagRetentionTime;
+ }
+
+ @Override
+ public void run() {
+ try {
+ log.info("start clean");
+ for (Iterator<Map.Entry<String, Dag<JobExecutionPlan>>> iter =
this.failedDags.entrySet().iterator(); iter.hasNext();) {
Review comment:
I think it is unsafe to remove elements from a collection that you are
iterating right? In the for loop I am removing elements from `failedDags`.
I did it this way as per
https://stackoverflow.com/questions/223918/iterating-through-a-collection-avoiding-concurrentmodificationexception-when-re,
although I realize now I still did it incorrectly, I should be calling
`iter.remove()` in the loop
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
