[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-05-07 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @srdo, You are right. This was exception I noticed on old internal version of the code package. It is clearly not necessary in 2.x latest code. Let me close this patch. ---

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-05-01 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2638 @kishorvpatil Sorry to keep harping on this, but I still don't really understand how we're getting `InterruptedExceptions` that need to be logged at error level. Could you give an example of a

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-30 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @srdo , What I mean is there are many instances where kafka is wrapping actual exceptions into `InterruptedException`. I am not sure why/what is the objective, but only way to understand the

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-27 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2638 @kishorvpatil Can you elaborate a bit? I don't understand why `TimeoutException` would be wrapped in an `InterruptedException`? Where is the log line you posted coming from, a grep of Storm didn't turn

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-23 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @HeartSaVioR @srdo ., The issue in assuming that `InterruptedException` is only coming from external interruptions. Below is the actual example we noticed when Kafka Spout`TimeoutException` was

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-21 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2638 +1, though I'm not sure whether it is necessary to change the log level to error. ---

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-19 Thread srdo
Github user srdo commented on the issue: https://github.com/apache/storm/pull/2638 Sorry if I'm nitpicking you to death, but I don't think moving the log line is better. Now everyone gets an error level log, even if the exception is due to an interrupt. I'd prefer if we just include

[GitHub] storm issue #2638: [STORM-3034] Adding exception stacktrace for executor fai...

2018-04-19 Thread kishorvpatil
Github user kishorvpatil commented on the issue: https://github.com/apache/storm/pull/2638 @srdo Moved the `LOG.error` before changing exception Cause. ---