[jira] [Comment Edited] (KYLIN-2858) Add retry in cache sync

["chaoli (JIRA)" <ji...@apache.org>]

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

chaoli edited comment on KYLIN-2858 at 9/9/17 7:09 PM:
-------------------------------------------------------

[~Shaofengshi]   hi , I found the reason why UT failed.

there are some code in the CacheServiceTest looks like: 


{code:java}

 assertEquals(1, broadcaster.getCounterAndClear());
 waitForCounterAndClear(1);
{code}

In fact, it's function is to make sure wipe cache request to reach  the cache service. 
And when wipe cache works fine, my retry strategy won't affect the UT result. But when the wipe cache failed, my strategy will put the wipe request back to broadcastEvent queue again. And that may
cause *waitForCounterAndClear* throw timeout exception. 
So I suggest the *waitForCounterAndClear* method refactor like this:


{code:java}
private void waitForCounterAndClear(long count) {
        int retryTimes = 0;
        while ((!counter.compareAndSet(count, 0L))) {
             // take into account wipe retry causing counter larger than count
            if(counter.get() > count){
                counter.decrementAndGet();
            }
            if (++retryTimes > 30) {
                throw new RuntimeException("timeout");
            }
            try {
                Thread.sleep(100L);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
{code}





was (Author: lichaojacobs):
[~Shaofengshi]   hi , I found the reason why UT failed.

there are some code in the CacheServiceTest looks like: 


{code:java}

 assertEquals(1, broadcaster.getCounterAndClear());
 waitForCounterAndClear(1);
{code}

In fact, it's function is to make sure wipe cache request to reach  the cache service. 
And when wipe cache works fine, my retry strategy won't affect the UT result. But when the wipe cache failed, my strategy will put the wipe request back to broadcastEvent queue again. And that may
cause *waitForCounterAndClear* throw timeout exception. 
So I suggest the *waitForCounterAndClear* method refactor like this:


{code:java}
private void waitForCounterAndClear(long count) {
        int retryTimes = 0;
        while ((!counter.compareAndSet(count, 0L))) {
             // take into account wipe retry causing counter larger than count
            if(counter.get() > count){
                counter.decrementAndGet();
            }
            if (++retryTimes > 30) {
                throw new RuntimeException("timeout"+counter.get());
            }
            try {
                Thread.sleep(100L);
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
    }
{code}




> Add retry in cache sync 
> ------------------------
>
>                 Key: KYLIN-2858
>                 URL: https://issues.apache.org/jira/browse/KYLIN-2858
>             Project: Kylin
>          Issue Type: Improvement
>          Components: Metadata
>    Affects Versions: v2.0.0, v2.1.0
>            Reporter: chaoli
>            Assignee: chaoli
>         Attachments: add_sync_retry.patch
>
>
> since there may be an situation that when sync request call a node that is down temporary and is restarting, sync cache may fail. So I add a retry strategy to avoid that situation



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)