[GitHub] [gobblin] umustafi commented on a diff in pull request #3658: [GOBBLIN-1798] Add backoff retry when we access mysql db for flow spec or dag action

["umustafi (via GitHub)" <gi...@apache.org>]

umustafi commented on code in PR #3658:
URL: https://github.com/apache/gobblin/pull/3658#discussion_r1131531300


##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/spec_catalog/FlowCatalog.java:
##########
@@ -82,12 +83,16 @@ public class FlowCatalog extends AbstractIdleService implements SpecCatalog, Mut
   public static final String DEFAULT_FLOWSPEC_STORE_CLASS = FSSpecStore.class.getCanonicalName();
   public static final String FLOWSPEC_SERDE_CLASS_KEY = "flowSpec.serde.class";
   public static final String DEFAULT_FLOWSPEC_SERDE_CLASS = JavaSpecSerDe.class.getCanonicalName();
+  public static final String FLOWCATALOG_GET_SPEC_MAX_RETRIES = "flowCatalog.get.spec.max.retries";
+  public static final int DEFAULT_FLOWCATALOG_GET_SPEC_MAX_RETRIES = 3;
+  private static final long FLOWCATALOG_GET_SPEC_INITIAL_WAIT_AFTER_FAILURE = 1000L;

Review Comment:
   same here, move to configurationkeys



##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/dag_action_store/MysqlDagActionStore.java:
##########
@@ -149,16 +155,29 @@ public DagAction getDagAction(String flowGroup, String flowName, String flowExec
       rs = getStatement.executeQuery();
       if (rs.next()) {
         return new DagAction(rs.getString(1), rs.getString(2), rs.getString(3), DagActionValue.valueOf(rs.getString(4)));
+      } else {
+        if (exponentialBackoff.awaitNextRetryIfAvailable()) {
+          return getDagActionWithRetry(flowGroup, flowName, flowExecutionId, exponentialBackoff);
+        } else {
+          return null;

Review Comment:
   do we want to add log if got null even with retry?



##########
gobblin-runtime/src/main/java/org/apache/gobblin/runtime/dag_action_store/MysqlDagActionStore.java:
##########
@@ -36,11 +36,15 @@
 import org.apache.gobblin.runtime.api.DagActionStore;
 import org.apache.gobblin.service.ServiceConfigKeys;
 import org.apache.gobblin.util.ConfigUtils;
+import org.apache.gobblin.util.ExponentialBackoff;
 
 
 public class MysqlDagActionStore implements DagActionStore {
 
   public static final String CONFIG_PREFIX = "MysqlDagActionStore";
+  public static final String GET_DAG_ACTION_MAX_RETRIES = "get.dagAction.max.retries";

Review Comment:
   should this be moved to configurationKeys, it is something you're proposing can be configured in config



-- 
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: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org