[
https://issues.apache.org/jira/browse/HADOOP-10768?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16476018#comment-16476018
]
Wei-Chiu Chuang commented on HADOOP-10768:
------------------------------------------
Here's my review comments. Some of them are already mentioned previously.
[~dapengsun] would you please update the patch if you have time available?
{quote}do you know why there's a ~50% degradation? That's concerning and may
severely impede performance (to the point I can't use it :()
{quote}
While that's the case, it is still 4~5x performant than existing
implementation, and I guess I'm still happy about that. The amount of memory
allocation is concerning. I'll try to find time to profile it today.
* CryptoInputStream
the new readFully() method is a public method but lacks javadoc.
is it different from the existing readFully() methods? Can you reuse the
existing readFully() method?
* Client
Why are we catching exceptions in Client.shouldAuthenticateOverKrb()? That
seems unnecessary. If not catching exception causes a problem, please have a
test case for it.
* SaslUtil
** negotiateCipherOption()
can you please throw a non IOException? I’m not in favor of making every
method throwing a generic IOExceptions. Similarly, update the method signature
for the code path (getCipherOption, processSaslToken)
This method is very similar to DataTransferSaslUtil#negotiateCipherOption()
except for the configuration keys.
I see no reason to duplicate the code, especially it involves some
coding/decoding, which is not that easy to comprehend.
** Also, every time this method is called, it returns a new List<>. I feel
like this is too much of a cost. Can we reduce the memory footprint?
* SaslRpcClient
** saslConnect()
LOG.debug(
"Get SASL RPC CipherOption from Conf" + cipherOptions);
—> missing a space after Conf
the method is well over 100 lines in length now. Some code refactor will
greatly improve readability.
** handleSaslCipherOptions()
throw new SaslException(e.getMessage(), e);
the exception message should be more descriptive. It could be something like
“Unable to initialize SaslCryptoCodec”
> Optimize Hadoop RPC encryption performance
> ------------------------------------------
>
> Key: HADOOP-10768
> URL: https://issues.apache.org/jira/browse/HADOOP-10768
> Project: Hadoop Common
> Issue Type: Improvement
> Components: performance, security
> Affects Versions: 3.0.0-alpha1
> Reporter: Yi Liu
> Assignee: Dapeng Sun
> Priority: Major
> Attachments: HADOOP-10768.001.patch, HADOOP-10768.002.patch,
> HADOOP-10768.003.patch, HADOOP-10768.004.patch, HADOOP-10768.005.patch,
> HADOOP-10768.006.patch, HADOOP-10768.007.patch, HADOOP-10768.008.patch,
> HADOOP-10768.009.patch, Optimize Hadoop RPC encryption performance.pdf
>
>
> Hadoop RPC encryption is enabled by setting {{hadoop.rpc.protection}} to
> "privacy". It utilized SASL {{GSSAPI}} and {{DIGEST-MD5}} mechanisms for
> secure authentication and data protection. Even {{GSSAPI}} supports using
> AES, but without AES-NI support by default, so the encryption is slow and
> will become bottleneck.
> After discuss with [~atm], [~tucu00] and [~umamaheswararao], we can do the
> same optimization as in HDFS-6606. Use AES-NI with more than *20x* speedup.
> On the other hand, RPC message is small, but RPC is frequent and there may be
> lots of RPC calls in one connection, we needs to setup benchmark to see real
> improvement and then make a trade-off.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
