[GitHub] [incubator-gobblin] autumnust commented on a change in pull request #2656: [GOBBLIN-790]DagStateStore MySQL

[GitBox <gi...@apache.org>]

autumnust commented on a change in pull request #2656: [GOBBLIN-790]DagStateStore MySQL
URL: https://github.com/apache/incubator-gobblin/pull/2656#discussion_r292074478
 
 

 ##########
 File path: gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagManager.java
 ##########
 @@ -582,11 +581,18 @@ private void cleanUp() {
       }
     }
 
-    private void cleanUpDag(String dagId) {
-      Dag<JobExecutionPlan> dag = this.dags.get(dagId);
-      this.dagToJobs.remove(dagId);
-      this.dagStateStore.cleanUp(dag);
+    /**
+     * Cleaning of all relevant states need to be
+     * @param dagId
+     */
+    private synchronized void cleanUpDag(String dagId) {
+       try {
+         this.dagStateStore.cleanUp(dags.get(dagId));
+       } catch (IOException ioe) {
+         log.error(String.format("Failed to clean %s from backStore due to:", dagId), ioe);
 
 Review comment:
   I believe `log.error (String, Exception)` will format the line better. What is the reason to have them cancatted as single string ? 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services