You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@gobblin.apache.org by GitBox <gi...@apache.org> on 2020/06/19 00:30:53 UTC

[GitHub] [incubator-gobblin] arekusuri opened a new pull request #3049: GOBBLIN-1202 & DSS-26966: Add retry for REST API call

arekusuri opened a new pull request #3049:
URL: https://github.com/apache/incubator-gobblin/pull/3049


   Dear Gobblin maintainers,
   
   
   ### JIRA
   https://issues.apache.org/jira/browse/GOBBLIN-1202
   
   ### Description
   SFDC objects have index on their column - SystemModstamp
   This index could be in disk. When we execute  
   **Select count(systemmodstamp) from table_name group by day_only(systemmodstamp)**
   If the index is in disk, it needs to load. It would be timeout.
   Retry would result it.
   


----------------------------------------------------------------
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



[GitHub] [incubator-gobblin] asfgit closed pull request #3049: GOBBLIN-1202 & DSS-26966: Add retry for REST API call

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #3049:
URL: https://github.com/apache/incubator-gobblin/pull/3049


   


----------------------------------------------------------------
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



[GitHub] [incubator-gobblin] arekusuri commented on a change in pull request #3049: GOBBLIN-1202 & DSS-26966: Add retry for REST API call

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #3049:
URL: https://github.com/apache/incubator-gobblin/pull/3049#discussion_r443789518



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java
##########
@@ -417,23 +425,31 @@ private int computeTargetPartitionSize(Histogram histogram, int minTargetPartiti
    * Get a {@link JsonArray} containing the query results
    */
   private JsonArray getRecordsForQuery(SalesforceConnector connector, String query) {
-    try {
-      String soqlQuery = SalesforceExtractor.getSoqlUrl(query);
-      List<Command> commands = RestApiConnector.constructGetCommand(connector.getFullUri(soqlQuery));
-      CommandOutput<?, ?> response = connector.getResponse(commands);
+    RestApiProcessingException exception = null;
+    for (int i = 0; i < workUnitConf.restRetries; i++) {
+      try {
+        String soqlQuery = SalesforceExtractor.getSoqlUrl(query);
+        List<Command> commands = RestApiConnector.constructGetCommand(connector.getFullUri(soqlQuery));
+        CommandOutput<?, ?> response = connector.getResponse(commands);
+
+        String output;
+        Iterator<String> itr = (Iterator<String>) response.getResults().values().iterator();
+        if (itr.hasNext()) {
+          output = itr.next();
+        } else {
+          throw new DataRecordException("Failed to get data from salesforce; REST response has no output");
+        }
 
-      String output;
-      Iterator<String> itr = (Iterator<String>) response.getResults().values().iterator();
-      if (itr.hasNext()) {
-        output = itr.next();
-      } else {
-        throw new DataRecordException("Failed to get data from salesforce; REST response has no output");
+        return GSON.fromJson(output, JsonObject.class).getAsJsonArray("records");
+      } catch (RestApiClientException | DataRecordException e) {
+        throw new RuntimeException("Fail to get data from salesforce", e);
+      } catch (RestApiProcessingException e) {
+        exception = e;
+        log.info(String.format("Caught RestApiProcessingException, retry(%s) rest query: %s", i+1, query));

Review comment:
       good catch, fixed!
   # And added wait duration
   # refactored variable name




----------------------------------------------------------------
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



[GitHub] [incubator-gobblin] arekusuri commented on a change in pull request #3049: GOBBLIN-1202 & DSS-26966: Add retry for REST API call

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #3049:
URL: https://github.com/apache/incubator-gobblin/pull/3049#discussion_r443790373



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java
##########
@@ -32,6 +32,7 @@
 import java.util.Set;
 
 import java.util.stream.Collectors;
+import lombok.SneakyThrows;

Review comment:
       Thanks! It was unused import. 
   We are using it now.




----------------------------------------------------------------
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



[GitHub] [incubator-gobblin] htran1 commented on a change in pull request #3049: GOBBLIN-1202 & DSS-26966: Add retry for REST API call

Posted by GitBox <gi...@apache.org>.
htran1 commented on a change in pull request #3049:
URL: https://github.com/apache/incubator-gobblin/pull/3049#discussion_r443276103



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java
##########
@@ -417,23 +425,31 @@ private int computeTargetPartitionSize(Histogram histogram, int minTargetPartiti
    * Get a {@link JsonArray} containing the query results
    */
   private JsonArray getRecordsForQuery(SalesforceConnector connector, String query) {
-    try {
-      String soqlQuery = SalesforceExtractor.getSoqlUrl(query);
-      List<Command> commands = RestApiConnector.constructGetCommand(connector.getFullUri(soqlQuery));
-      CommandOutput<?, ?> response = connector.getResponse(commands);
+    RestApiProcessingException exception = null;
+    for (int i = 0; i < workUnitConf.restRetries; i++) {
+      try {
+        String soqlQuery = SalesforceExtractor.getSoqlUrl(query);
+        List<Command> commands = RestApiConnector.constructGetCommand(connector.getFullUri(soqlQuery));
+        CommandOutput<?, ?> response = connector.getResponse(commands);
+
+        String output;
+        Iterator<String> itr = (Iterator<String>) response.getResults().values().iterator();
+        if (itr.hasNext()) {
+          output = itr.next();
+        } else {
+          throw new DataRecordException("Failed to get data from salesforce; REST response has no output");
+        }
 
-      String output;
-      Iterator<String> itr = (Iterator<String>) response.getResults().values().iterator();
-      if (itr.hasNext()) {
-        output = itr.next();
-      } else {
-        throw new DataRecordException("Failed to get data from salesforce; REST response has no output");
+        return GSON.fromJson(output, JsonObject.class).getAsJsonArray("records");
+      } catch (RestApiClientException | DataRecordException e) {
+        throw new RuntimeException("Fail to get data from salesforce", e);
+      } catch (RestApiProcessingException e) {
+        exception = e;
+        log.info(String.format("Caught RestApiProcessingException, retry(%s) rest query: %s", i+1, query));

Review comment:
       Please use {} logger formatting instead of String.format.

##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java
##########
@@ -32,6 +32,7 @@
 import java.util.Set;
 
 import java.util.stream.Collectors;
+import lombok.SneakyThrows;

Review comment:
       Unused import?




----------------------------------------------------------------
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



[GitHub] [incubator-gobblin] arekusuri commented on a change in pull request #3049: GOBBLIN-1202 & DSS-26966: Add retry for REST API call

Posted by GitBox <gi...@apache.org>.
arekusuri commented on a change in pull request #3049:
URL: https://github.com/apache/incubator-gobblin/pull/3049#discussion_r443789518



##########
File path: gobblin-salesforce/src/main/java/org/apache/gobblin/salesforce/SalesforceSource.java
##########
@@ -417,23 +425,31 @@ private int computeTargetPartitionSize(Histogram histogram, int minTargetPartiti
    * Get a {@link JsonArray} containing the query results
    */
   private JsonArray getRecordsForQuery(SalesforceConnector connector, String query) {
-    try {
-      String soqlQuery = SalesforceExtractor.getSoqlUrl(query);
-      List<Command> commands = RestApiConnector.constructGetCommand(connector.getFullUri(soqlQuery));
-      CommandOutput<?, ?> response = connector.getResponse(commands);
+    RestApiProcessingException exception = null;
+    for (int i = 0; i < workUnitConf.restRetries; i++) {
+      try {
+        String soqlQuery = SalesforceExtractor.getSoqlUrl(query);
+        List<Command> commands = RestApiConnector.constructGetCommand(connector.getFullUri(soqlQuery));
+        CommandOutput<?, ?> response = connector.getResponse(commands);
+
+        String output;
+        Iterator<String> itr = (Iterator<String>) response.getResults().values().iterator();
+        if (itr.hasNext()) {
+          output = itr.next();
+        } else {
+          throw new DataRecordException("Failed to get data from salesforce; REST response has no output");
+        }
 
-      String output;
-      Iterator<String> itr = (Iterator<String>) response.getResults().values().iterator();
-      if (itr.hasNext()) {
-        output = itr.next();
-      } else {
-        throw new DataRecordException("Failed to get data from salesforce; REST response has no output");
+        return GSON.fromJson(output, JsonObject.class).getAsJsonArray("records");
+      } catch (RestApiClientException | DataRecordException e) {
+        throw new RuntimeException("Fail to get data from salesforce", e);
+      } catch (RestApiProcessingException e) {
+        exception = e;
+        log.info(String.format("Caught RestApiProcessingException, retry(%s) rest query: %s", i+1, query));

Review comment:
       good catch, fixed!
   * And added wait duration
   * refactored variable name




----------------------------------------------------------------
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