You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@flume.apache.org by "Denes Arvay (JIRA)" <ji...@apache.org> on 2017/01/31 16:53:51 UTC
[jira] (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:all-tabpanel ]
Denes Arvay updated FLUME-3049:
-------------------------------
Description:
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.
was:
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.
> 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)