[
https://issues.apache.org/jira/browse/HADOOP-14443?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16020980#comment-16020980
]
Steve Loughran commented on HADOOP-14443:
-----------------------------------------
# I've not reviewed the actual functionality as that goes near UGI and the
like. Someone who understands that will need to review it.
# We have the same policy for Azure patches as for the other object stores: can
you state which endpoint you tested against (e.g . Azure ireland)
# hit the "submit patch" for Yetus to review
General
* the Line ending style checker is going to be unhappy. Please cut down where
it doesn't destroy readability? Why: helps side-by-side review.
* embrace {{Configuration.getTrimmedStrings()}}
* SL4J construct strings automatically; critical for performance of debug log
messages. Switch to {{LOG.debug("connecting to {}", uri)}} structure.
h4. {{RemoteSASKeyGeneratorImpl}}
L124
{code}
commaSeparatedUrlsString = conf.get(KEY_CRED_SERVICE_URLS);
{code}
this should use conf.getTrimmedStrings() to have whitespace stripped, split
done, tests for all this. Same for {{RemoteWasbAuthorizerImpl}} and
{{RemoteWasbDelegationTokenManager}}
L163: can you include the URI at fault in the exception text
h4. {{RemoteWasbAuthorizerImpl}}
L157: Unless its always in inner messages, can you somehow include the
URI/endpoint details in the wrapped exception. Your support team will
appreciate this.
L169: time to use {{<pre>}} in the javadocs.
h4. {{SecureWasbRemoteCallHelper}}
L143. Why not make that commented out LOG.info an uncommented LOG.debug?
L159. Use try-with-resources to manage closing of response
L174/175. Log message at WARN, print the stack at DEBUG
L217: SL4J construcst strings automatically; critical for performance of debug
log messages. Switch to {{"connecting to {}", uri}} style.
L225. Catching of any Exception is overbroad. Maybe: all IOEs pass up as is.
h4. {{JsonUtils}}
L42. again, {{LOG.debug("JSON Parsing exception: {}", e.getMessage())}}
L42. Maybe: log@debug the errant JSON string
L43. String.toLowerCase needs to specify {{Locale.EN}} to work reliably round
the world
> Azure: Add retry and client side failover for authorization, SASKey
> generation and delegation token generation requests to remote service
> -----------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HADOOP-14443
> URL: https://issues.apache.org/jira/browse/HADOOP-14443
> Project: Hadoop Common
> Issue Type: Improvement
> Components: fs/azure
> Affects Versions: 2.9.0
> Reporter: Santhosh G Nayak
> Assignee: Santhosh G Nayak
> Fix For: 2.9.0, 3.0.0-alpha3
>
> Attachments: HADOOP-14443.1.patch
>
>
> Currently, {{WasRemoteCallHelper}} can be configured to talk to only one URL
> for authorization, SASKey generation and delegation token generation. If for
> some reason the service is down, all the requests will fail.
> So proposal is to,
> - Add support to configure multiple URLs, so that if communication to one URL
> fails, client can retry on another instance of the service running on
> different node for authorization, SASKey generation and delegation token
> generation.
> - Rename the configurations {{fs.azure.authorization.remote.service.url}} to
> {{fs.azure.authorization.remote.service.urls}} and
> {{fs.azure.cred.service.url}} to {{fs.azure.cred.service.urls}} to support
> the comma separated list of URLs.
> Introduce a new configuration {{fs.azure.delegation.token.service.urls}} to
> configure the comma separated list of service URLs to get the delegation
> token.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
