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 2021/05/26 21:57:46 UTC

[GitHub] [gobblin] aplex opened a new pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

aplex opened a new pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289


   Previously, the root cause of the problems were wrapped in several
   container exceptions that added little value in troubleshooting and
   confused the users. They frequently reported that the job failed with
   "RetryException" without looking at the root cause of it.
   
   With this change, we'll unwrap the exceptions from tasks and show
   the possible root cause at the top of the error message.
   
   https://issues.apache.org/jira/browse/GOBBLIN-1451
   


-- 
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] [gobblin] sv2000 commented on a change in pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#discussion_r657370037



##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -24,6 +24,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.github.rholder.retry.RetryException;

Review comment:
       Unused import.

##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -50,12 +51,14 @@
 import org.apache.gobblin.records.RecordStreamWithMetadata;
 import org.apache.gobblin.runtime.BoundedBlockingRecordQueue;
 import org.apache.gobblin.runtime.ExecutionModel;
+import org.apache.gobblin.runtime.ForkException;

Review comment:
       Unused import.

##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/util/ExceptionCleanupUtils.java
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.runtime.util;
+
+public final class ExceptionCleanupUtils {
+
+  private ExceptionCleanupUtils() {
+  }
+
+  /**
+   * Removes exceptions that were wrapping another exception without providing a message of their own.
+   *
+   * When a checked exception is defined on the interface, and one of the implementations want to propagate a
+   * different type of exception, that implementation can wrap the original exception. For example, Gobblin
+   * codebase frequently wraps exception into new IOException(cause). As a result, users see large stack traces
+   * where real error is hidden below several wrappers. This method will remove the wrappers to provide users with
+   * better error messages.
+   * */
+  public static Throwable removeEmptyWrappers(Throwable exception) {
+    if (exception == null) {
+      return null;
+    }
+
+    if (exception.getCause() != null && exception.getCause().toString().equals(exception.getMessage())) {
+      return removeEmptyWrappers(exception.getCause());
+    }
+
+    return exception;
+  }
+  /**
+   * The method will return the bottom of the exception stack, up to one of the provided wrappers.
+   *
+   * It is useful when exception can be wrapped multiple times in the codebase, and only some of those wrappings
+   * can contain useful information, while others will just confuse the user.
+   *
+   * Examples: if the chain is wrapper1 -> wrapper2 -> real1 -> real2, the method will return real1 -> real2 chain.
+   * */
+  public static Throwable getRootCauseUpToWrapper(Throwable exception, Class<?>... wrapperExceptionTypes) {

Review comment:
       Is this method used anywhere other than test class?




-- 
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] [gobblin] asfgit closed pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

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


   


-- 
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] [gobblin] aplex commented on pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
aplex commented on pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#issuecomment-849780149


   tagging @jack-moseley , @sv2000 


-- 
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] [gobblin] codecov-commenter commented on pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#issuecomment-849159352


   # [Codecov](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3289](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6a8e99d) into [master](https://codecov.io/gh/apache/gobblin/commit/569cdb762367a132e691936b341f5cea0c3047f1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (569cdb7) will **decrease** coverage by `37.51%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/gobblin/pull/3289/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master   #3289       +/-   ##
   ============================================
   - Coverage     46.53%   9.01%   -37.52%     
   + Complexity    10028    1748     -8280     
   ============================================
     Files          2037    2038        +1     
     Lines         79271   79289       +18     
     Branches       8839    8844        +5     
   ============================================
   - Hits          36886    7149    -29737     
   - Misses        38963   71438    +32475     
   + Partials       3422     702     -2720     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ain/java/org/apache/gobblin/runtime/fork/Fork.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvZm9yay9Gb3JrLmphdmE=) | `48.80% <100.00%> (-27.50%)` | :arrow_down: |
   | [...he/gobblin/runtime/util/ExceptionCleanupUtils.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvdXRpbC9FeGNlcHRpb25DbGVhbnVwVXRpbHMuamF2YQ==) | `100.00% <100.00%> (ø)` | |
   | [...c/main/java/org/apache/gobblin/util/FileUtils.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvRmlsZVV0aWxzLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...n/java/org/apache/gobblin/fork/CopyableSchema.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ZvcmsvQ29weWFibGVTY2hlbWEuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...java/org/apache/gobblin/stream/ControlMessage.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vc3RyZWFtL0NvbnRyb2xNZXNzYWdlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...va/org/apache/gobblin/dataset/DatasetResolver.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vZGF0YXNldC9EYXRhc2V0UmVzb2x2ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...va/org/apache/gobblin/converter/EmptyIterable.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbnZlcnRlci9FbXB0eUl0ZXJhYmxlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...org/apache/gobblin/ack/BasicAckableForTesting.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dvYmJsaW4vYWNrL0Jhc2ljQWNrYWJsZUZvclRlc3RpbmcuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...n/java/org/apache/gobblin/salesforce/SfConfig.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1zYWxlc2ZvcmNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NhbGVzZm9yY2UvU2ZDb25maWcuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../org/apache/gobblin/yarn/HelixMessageSubTypes.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi15YXJuL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3lhcm4vSGVsaXhNZXNzYWdlU3ViVHlwZXMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [1082 more](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [569cdb7...6a8e99d](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] [gobblin] sv2000 commented on a change in pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
sv2000 commented on a change in pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#discussion_r643401645



##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -265,11 +268,15 @@ public void run() {
 
       compareAndSetForkState(ForkState.RUNNING, ForkState.SUCCEEDED);
     } catch (Throwable t) {
+      Throwable rootCauseException =

Review comment:
       It looks like we are leaking information into the Fork class, making it aware of exception types such as ForkException and RetryException. Further, the RetryException is only thrown if a retriable writer is enabled. 

##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -265,11 +268,15 @@ public void run() {
 
       compareAndSetForkState(ForkState.RUNNING, ForkState.SUCCEEDED);
     } catch (Throwable t) {
+      Throwable rootCauseException =

Review comment:
       The other concern I have is if Fork is the right layer at which we should be unwrapping exceptions. Why not do this inside the Task class?  

##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -265,11 +268,15 @@ public void run() {
 
       compareAndSetForkState(ForkState.RUNNING, ForkState.SUCCEEDED);
     } catch (Throwable t) {
+      Throwable rootCauseException =

Review comment:
       Finally, we may want to preserve the wrapped exceptions as it shows the sequence in which the exceptions are propagated up, which is useful for debugging. The "likely" root cause exception can be logged in addition to the wrapped exception. 




-- 
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] [gobblin] codecov-commenter edited a comment on pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#issuecomment-849159352


   # [Codecov](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3289](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (358a95d) into [master](https://codecov.io/gh/apache/gobblin/commit/569cdb762367a132e691936b341f5cea0c3047f1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (569cdb7) will **decrease** coverage by `3.49%`.
   > The diff coverage is `80.00%`.
   
   > :exclamation: Current head 358a95d differs from pull request most recent head 6a8e99d. Consider uploading reports for the commit 6a8e99d to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/gobblin/pull/3289/graphs/tree.svg?width=650&height=150&src=pr&token=4MgURJ0bGc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #3289      +/-   ##
   ============================================
   - Coverage     46.53%   43.03%   -3.50%     
   + Complexity    10028     9307     -721     
   ============================================
     Files          2037     2038       +1     
     Lines         79271    79289      +18     
     Branches       8839     8844       +5     
   ============================================
   - Hits          36886    34124    -2762     
   - Misses        38963    41935    +2972     
   + Partials       3422     3230     -192     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...kunit/packer/KafkaTopicGroupingWorkUnitPacker.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4ta2Fma2EtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZXh0cmFjdC9rYWZrYS93b3JrdW5pdC9wYWNrZXIvS2Fma2FUb3BpY0dyb3VwaW5nV29ya1VuaXRQYWNrZXIuamF2YQ==) | `40.65% <0.00%> (-0.34%)` | :arrow_down: |
   | [...gobblin/iceberg/publisher/GobblinMCEPublisher.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1pY2ViZXJnL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ljZWJlcmcvcHVibGlzaGVyL0dvYmJsaW5NQ0VQdWJsaXNoZXIuamF2YQ==) | `66.66% <50.00%> (-1.69%)` | :arrow_down: |
   | [.../gobblin/iceberg/writer/IcebergMetadataWriter.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1pY2ViZXJnL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2ljZWJlcmcvd3JpdGVyL0ljZWJlcmdNZXRhZGF0YVdyaXRlci5qYXZh) | `68.93% <100.00%> (+1.36%)` | :arrow_up: |
   | [...lin/source/extractor/extract/kafka/KafkaUtils.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1tb2R1bGVzL2dvYmJsaW4ta2Fma2EtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3NvdXJjZS9leHRyYWN0b3IvZXh0cmFjdC9rYWZrYS9LYWZrYVV0aWxzLmphdmE=) | `61.11% <100.00%> (+2.28%)` | :arrow_up: |
   | [...ain/java/org/apache/gobblin/runtime/fork/Fork.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvZm9yay9Gb3JrLmphdmE=) | `75.79% <100.00%> (-0.52%)` | :arrow_down: |
   | [...he/gobblin/runtime/util/ExceptionCleanupUtils.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvdXRpbC9FeGNlcHRpb25DbGVhbnVwVXRpbHMuamF2YQ==) | `100.00% <100.00%> (ø)` | |
   | [.../org/apache/gobblin/util/filters/HiddenFilter.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi11dGlsaXR5L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3V0aWwvZmlsdGVycy9IaWRkZW5GaWx0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...g/apache/gobblin/cluster/HelixMessageSubTypes.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jbHVzdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NsdXN0ZXIvSGVsaXhNZXNzYWdlU3ViVHlwZXMuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...gobblin/runtime/mapreduce/GobblinOutputFormat.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1ydW50aW1lL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL3J1bnRpbWUvbWFwcmVkdWNlL0dvYmJsaW5PdXRwdXRGb3JtYXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [...obblin/compaction/source/CompactionFailedTask.java](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z29iYmxpbi1jb21wYWN0aW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9nb2JibGluL2NvbXBhY3Rpb24vc291cmNlL0NvbXBhY3Rpb25GYWlsZWRUYXNrLmphdmE=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | ... and [146 more](https://codecov.io/gh/apache/gobblin/pull/3289/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [569cdb7...6a8e99d](https://codecov.io/gh/apache/gobblin/pull/3289?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
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] [gobblin] aplex commented on a change in pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
aplex commented on a change in pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#discussion_r657388441



##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -24,6 +24,7 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.github.rholder.retry.RetryException;

Review comment:
       That should have been caught by checkstyle plugin, but apparently it is configured to check only a very small subset of things. I'll send separate PRs to enable more checks.




-- 
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] [gobblin] aplex commented on a change in pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
aplex commented on a change in pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#discussion_r647944959



##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/fork/Fork.java
##########
@@ -265,11 +268,15 @@ public void run() {
 
       compareAndSetForkState(ForkState.RUNNING, ForkState.SUCCEEDED);
     } catch (Throwable t) {
+      Throwable rootCauseException =

Review comment:
       Refactored the code to clean up exceptions at lower layers.




-- 
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] [gobblin] aplex commented on a change in pull request #3289: [GOBBLIN-1451] Log better error messages from mappers

Posted by GitBox <gi...@apache.org>.
aplex commented on a change in pull request #3289:
URL: https://github.com/apache/gobblin/pull/3289#discussion_r657390969



##########
File path: gobblin-runtime/src/main/java/org/apache/gobblin/runtime/util/ExceptionCleanupUtils.java
##########
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.gobblin.runtime.util;
+
+public final class ExceptionCleanupUtils {
+
+  private ExceptionCleanupUtils() {
+  }
+
+  /**
+   * Removes exceptions that were wrapping another exception without providing a message of their own.
+   *
+   * When a checked exception is defined on the interface, and one of the implementations want to propagate a
+   * different type of exception, that implementation can wrap the original exception. For example, Gobblin
+   * codebase frequently wraps exception into new IOException(cause). As a result, users see large stack traces
+   * where real error is hidden below several wrappers. This method will remove the wrappers to provide users with
+   * better error messages.
+   * */
+  public static Throwable removeEmptyWrappers(Throwable exception) {
+    if (exception == null) {
+      return null;
+    }
+
+    if (exception.getCause() != null && exception.getCause().toString().equals(exception.getMessage())) {
+      return removeEmptyWrappers(exception.getCause());
+    }
+
+    return exception;
+  }
+  /**
+   * The method will return the bottom of the exception stack, up to one of the provided wrappers.
+   *
+   * It is useful when exception can be wrapped multiple times in the codebase, and only some of those wrappings
+   * can contain useful information, while others will just confuse the user.
+   *
+   * Examples: if the chain is wrapper1 -> wrapper2 -> real1 -> real2, the method will return real1 -> real2 chain.
+   * */
+  public static Throwable getRootCauseUpToWrapper(Throwable exception, Class<?>... wrapperExceptionTypes) {

Review comment:
       It was used before, but is not used now. Removed to have less code.




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