[ 
https://issues.apache.org/jira/browse/HDFS-14088?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16694217#comment-16694217
 ] 

Yuxuan Wang commented on HDFS-14088:
------------------------------------

Thanks Íñigo Goiri for reviewing.
Here's a new patch HDFS-14088.002.patch for trunk.

I'm sorry about not explaining clearly. The patch avoid synchronize the whole 
thing with double check locking.

The original question is, after calling performFailover() and getProxy(), the 
former RequestHedgingInvocationHandler instances should be deprecated can still 
access currentUsedProxy. It can cause the handler success pass the if-condition 
{code:java}
currentUsedProxy != null
{code} 
 but immediately it turn to null because of performFailover() called by 
somebody.

My idea is, let the RequestHedgingInvocationHandler hold the currentUsedProxy 
and RequestHedgingProxyProvider hold the currentUsedHandler which warp the 
RequestHedgingInvocationHandler into a proxy like before. Fail over will set 
currentUsedHandler to null and getProxy() will assign a new 
RequestHedgingInvocationHandler to it and return. So we can avoid the 
deprecated handler can still access null currentUsedProxy after calling 
performFailover().

For the unit test, I add some metric check. The test's idea is mock a call and 
sleep, then call performFailover(). The original code 
{code:java}
if (currentUsedProxy != null) {
  try {
    Object retVal = method.invoke(currentUsedProxy.proxy, args);
    LOG.debug("Invocation successful on [{}]",
        currentUsedProxy.proxyInfo);
    return retVal;
  } 
{code}
debug log can throw a NullPointerException.
I know the idea for unit test is a little tricky, but I can't figure out better 
one.

Reformat the code.

> RequestHedgingProxyProvider can throw NullPointerException when failover due 
> to no lock on currentUsedProxy
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: HDFS-14088
>                 URL: https://issues.apache.org/jira/browse/HDFS-14088
>             Project: Hadoop HDFS
>          Issue Type: Bug
>          Components: hdfs-client
>            Reporter: Yuxuan Wang
>            Assignee: Yuxuan Wang
>            Priority: Major
>         Attachments: HDFS-14088.001.patch, HDFS-14088.002.patch
>
>
> {code:java}
> if (currentUsedProxy != null) {
>         try {
>           Object retVal = method.invoke(currentUsedProxy.proxy, args);
>           LOG.debug("Invocation successful on [{}]",
>               currentUsedProxy.proxyInfo);
> {code}
> If a thread run try block and then other thread trigger a fail over calling 
> method
> {code:java}
> @Override
>   public synchronized void performFailover(T currentProxy) {
>     toIgnore = this.currentUsedProxy.proxyInfo;
>     this.currentUsedProxy = null;
>   }
> {code}
> It will set currentUsedProxy to null, and the first thread can throw a 
> NullPointerException.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: hdfs-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: hdfs-issues-h...@hadoop.apache.org

Reply via email to