[ 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: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org