[GitHub] [gobblin] ZihanLi58 commented on a diff in pull request #3498: [GOBBLIN-1637] Add writer, operation, and partition info to failed metadata writer events

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

ZihanLi58 commented on code in PR #3498:
URL: https://github.com/apache/gobblin/pull/3498#discussion_r864282230


##########
gobblin-hive-registration/src/main/java/org/apache/gobblin/hive/writer/HiveMetadataWriter.java:
##########
@@ -114,8 +118,12 @@ public void flush(String dbName, String tableName) throws IOException {
       for (HashMap.Entry<List<String>, ListenableFuture<Void>> execution : executionMap.entrySet()) {
         try {
           execution.getValue().get(timeOutSeconds, TimeUnit.SECONDS);
-        } catch (InterruptedException | ExecutionException | TimeoutException e) {
-          throw new RuntimeException("Error when getting the result of registration for table" + tableKey, e);
+        } catch (TimeoutException e) {
+          // Since TimeoutException should always be a transient issue, throw RuntimeException which will fail/retry container
+          throw new RuntimeException("Timeout waiting for result of registration for table " + tableKey, e);
+        } catch (InterruptedException | ExecutionException e) {
+          Set<String> partitions = executionMap.keySet().stream().flatMap(List::stream).collect(Collectors.toSet());
+          throw new HiveMetadataWriterWithPartitionInfoException(partitions, Collections.emptySet(), e);
         }
       }
       executionMap.clear();

Review Comment:
   Do we want to clean the executionMap when we have exception?



##########
gobblin-iceberg/src/main/java/org/apache/gobblin/iceberg/writer/GobblinMCEWriter.java:
##########
@@ -331,7 +336,7 @@ void writeWithMetadataWriters(
         } catch (Exception e) {
           meetException = true;
           writer.reset(dbName, tableName);
-          addOrThrowException(e, tableString, dbName, tableName);
+          addOrThrowException(e, tableString, dbName, tableName, writer.getClass().getName());

Review Comment:
   Since we disable the following writer as well, not sure whether you want to add the following writer to the exception as well, depends on you



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