[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15848850#comment-15848850
]
Hudson commented on FLUME-3049:
---
ABORTED: Integrated in Jenkins build Flume-trunk-hbase-1 #239 (See
[https://builds.apache.org/job/Flume-trunk-hbase-1/239/])
FLUME-3049. Make HDFS sink rotate more reliably in secure mode (mpercy:
[http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=f215374bdf9a08b16fa7ccd3b40098024afe8677])
* (edit)
flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestBucketWriter.java
* (edit) flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java
* (edit)
flume-ng-auth/src/test/java/org/apache/flume/auth/TestFlumeAuthenticator.java
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>Assignee: Denes Arvay
> Fix For: v1.8.0
>
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> Here IOException is considered as non-transient failure thus the {{close()}}
> is called, but when the original exception is wrapped into
> {{SecurityException}} it doesn't trigger the close of the file.
> Similarly in {{HDFSEventSink.process()}} method the `IOException` is handled
> in a different way than any other exception. It might come from
> {{BucketWriter.append()}} or {{BucketWriter.flush()}} for example, and both
> of them invoke the hdfs call via a {{PrivilegedExecutor}} instance which
> might be the problematic {{UGIExecutor}}.
> The bottom line is that the contract in {{PrivilegedExecutor.execute()}} is
> that they shouldn't change the exception thrown in the business logic - at
> least it's not indicated in its signature in any way. The default
> implementation ({{SimpleAuthenticator}}) behaves according to this.
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15848694#comment-15848694
]
ASF subversion and git services commented on FLUME-3049:
Commit f215374bdf9a08b16fa7ccd3b40098024afe8677 in flume's branch
refs/heads/trunk from [~denes]
[ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=f215374 ]
FLUME-3049. Make HDFS sink rotate more reliably in secure mode
It was reported that the HDFS sink had a bug where file rotation was not
reliable in secure mode.
After investigating, it turns out that this was caused by a bug in the
FlumeAuthenticator code: A "try" block in UGIExecutor.execute() was
wrapping exceptions (such as IOException) with a SecurityException.
That exception wrapping was breaking the contract of BucketWriter
because the caller (HDFSEventSink) did not understand how to react to
the SecurityException. This also likely had other negative effects in
exceptional cases.
The following changes are included in this patch:
* Remove the exception wrapping in UGIExecutor.execute().
* Add tests for exception propagation in FlumeAuthenticator
implementations.
* Add testRotateBucketOnIOException() to TestBucketWriter as a
regression test for the HDFS sink issue.
Closes #106.
Reviewers: Attila Simon, Mike Percy
(Denes Arvay via Mike Percy)
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>Assignee: Denes Arvay
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> Here IOException is considered as non-transient failure thus the {{close()}}
> is called, but when the original exception is wrapped into
> {{SecurityException}} it doesn't trigger the close of the file.
> Similarly in {{HDFSEventSink.process()}} method the `IOException` is handled
> in a different way than any other exception. It might come from
> {{BucketWriter.append()}} or {{BucketWriter.flush()}} for example, and both
> of them invoke the hdfs call via a {{PrivilegedExecutor}} instance which
> might be the problematic {{UGIExecutor}}.
> The bottom line is that the contract in {{PrivilegedExecutor.execute()}} is
> that they shouldn't change the exception thrown in the business logic - at
> least it's not indicated in its signature in any way. The default
> implementation ({{SimpleAuthenticator}}) behaves according to this.
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15848695#comment-15848695
]
ASF GitHub Bot commented on FLUME-3049:
---
Github user asfgit closed the pull request at:
https://github.com/apache/flume/pull/106
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>Assignee: Denes Arvay
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> Here IOException is considered as non-transient failure thus the {{close()}}
> is called, but when the original exception is wrapped into
> {{SecurityException}} it doesn't trigger the close of the file.
> Similarly in {{HDFSEventSink.process()}} method the `IOException` is handled
> in a different way than any other exception. It might come from
> {{BucketWriter.append()}} or {{BucketWriter.flush()}} for example, and both
> of them invoke the hdfs call via a {{PrivilegedExecutor}} instance which
> might be the problematic {{UGIExecutor}}.
> The bottom line is that the contract in {{PrivilegedExecutor.execute()}} is
> that they shouldn't change the exception thrown in the business logic - at
> least it's not indicated in its signature in any way. The default
> implementation ({{SimpleAuthenticator}}) behaves according to this.
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15848690#comment-15848690
]
ASF subversion and git services commented on FLUME-3049:
Commit f215374bdf9a08b16fa7ccd3b40098024afe8677 in flume's branch
refs/heads/flume-3049-3 from [~denes]
[ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=f215374 ]
FLUME-3049. Make HDFS sink rotate more reliably in secure mode
It was reported that the HDFS sink had a bug where file rotation was not
reliable in secure mode.
After investigating, it turns out that this was caused by a bug in the
FlumeAuthenticator code: A "try" block in UGIExecutor.execute() was
wrapping exceptions (such as IOException) with a SecurityException.
That exception wrapping was breaking the contract of BucketWriter
because the caller (HDFSEventSink) did not understand how to react to
the SecurityException. This also likely had other negative effects in
exceptional cases.
The following changes are included in this patch:
* Remove the exception wrapping in UGIExecutor.execute().
* Add tests for exception propagation in FlumeAuthenticator
implementations.
* Add testRotateBucketOnIOException() to TestBucketWriter as a
regression test for the HDFS sink issue.
Closes #106.
Reviewers: Attila Simon, Mike Percy
(Denes Arvay via Mike Percy)
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>Assignee: Denes Arvay
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> Here IOException is considered as non-transient failure thus the {{close()}}
> is called, but when the original exception is wrapped into
> {{SecurityException}} it doesn't trigger the close of the file.
> Similarly in {{HDFSEventSink.process()}} method the `IOException` is handled
> in a different way than any other exception. It might come from
> {{BucketWriter.append()}} or {{BucketWriter.flush()}} for example, and both
> of them invoke the hdfs call via a {{PrivilegedExecutor}} instance which
> might be the problematic {{UGIExecutor}}.
> The bottom line is that the contract in {{PrivilegedExecutor.execute()}} is
> that they shouldn't change the exception thrown in the business logic - at
> least it's not indicated in its signature in any way. The default
> implementation ({{SimpleAuthenticator}}) behaves according to this.
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15842561#comment-15842561
]
Denes Arvay commented on FLUME-3049:
Thanks [~hshreedharan], I have opened a pull request for this change.
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>Assignee: Denes Arvay
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15842559#comment-15842559
]
ASF GitHub Bot commented on FLUME-3049:
---
GitHub user adenes opened a pull request:
https://github.com/apache/flume/pull/106
FLUME-3049: Remove exception wrapping in UGIExecutor.execute()
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/adenes/flume FLUME-3049
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flume/pull/106.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #106
commit a4286afc5eb28cb7d89137a53bfe47c365e5a170
Author: Denes Arvay
Date: 2017-01-27T10:43:27Z
FLUME-3049: Remove exception wrapping in UGIExecutor.execute()
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>Assignee: Denes Arvay
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (FLUME-3049) Wrapping the exception into SecurityException in UGIExecutor.execute hides the original one
[
https://issues.apache.org/jira/browse/FLUME-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15840091#comment-15840091
]
Hari Shreedharan commented on FLUME-3049:
-
I think that is a bug and was an oversight when I reviewed it. I think we
should throw the actual exception itself. Please go ahead and throw the actual
exception and removing the {{SecurityException}}
> Wrapping the exception into SecurityException in UGIExecutor.execute hides
> the original one
> ---
>
> Key: FLUME-3049
> URL: https://issues.apache.org/jira/browse/FLUME-3049
> Project: Flume
> Issue Type: Bug
>Reporter: Denes Arvay
>
> see:
> https://github.com/apache/flume/blob/trunk/flume-ng-auth/src/main/java/org/apache/flume/auth/UGIExecutor.java#L49
> This has unexpected side effects as the callers try to catch the wrapped
> exception, for example in {{BucketWriter.append()}}:
> https://github.com/apache/flume/blob/trunk/flume-ng-sinks/flume-hdfs-sink/src/main/java/org/apache/flume/sink/hdfs/BucketWriter.java#L563
> I don't know the original intend behind this wrapping, [~jrufus] or
> [~hshreedharan], do you happen to remember? (You were involved in the
> original implementation in FLUME-2631)
> Right now I don't see any problem in removing this and letting the original
> exception to propagate as the {{org.apache.flume.auth.SecurityException}}
> doesn't appear anywhere in the public interface.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
