[jira] [Commented] (HBASE-7384) Introducing waitForCondition function into test cases

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

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

Enis Soztutar commented on HBASE-7384:
--------------------------------------

Some comments below:

1. Can you change: 
{code}
+  public static final String TEST_WAITFOR_RATIO_PROP = "test.waitfor.ratio";
{code}
to "hbase.test.wait.for.ratio" (same for WAIT_FOR_RATIO_DEFAULT)
2. Waiter does not have InterfaceAudience annotation. Can you add @InterfaceAudience.Private
3. For Predicate, can we instead use com.google.collections.Predicate, or do we need the exception in method signature. If we want to allow Predicate.evaluate() to throw exceptions, then we want to bubble this up to the waitFor(), and eventually to the test which called waitFor(). We can define the exception type as a generic, and allow for waitFor to throw this as well. Smt like:
{code}
class  Predicate<E> { boolean evaluate() throws E }
public static <E> long waitFor(long timeout, long interval, Predicate<E> predicate) throws E
{code}

4.Predicate should not be annotated Public. 
5. Why are we evaluating the predicate on interruption? 
6. Can you get the hbase.test.wait.for.ratio from configuration instead of system property. We can either get the configuration from the method definitions, or maybe hook into HBaseTestingUtility.

                
> Introducing waitForCondition function into test cases
> -----------------------------------------------------
>
>                 Key: HBASE-7384
>                 URL: https://issues.apache.org/jira/browse/HBASE-7384
>             Project: HBase
>          Issue Type: Test
>          Components: test
>            Reporter: Jeffrey Zhong
>            Assignee: Jeffrey Zhong
>              Labels: test
>             Fix For: 0.96.0
>
>         Attachments: hbase-7384_1.0.patch, hbase-7384.patch, Waiter.java
>
>
> Recently I'm working on flaky test cases and found we have many places using while loop and sleep to wait for a condition to be true. There are several issues in existing ways:
> 1) Many similar code doing the same thing
> 2) When time out happens, different errors are reported without explicitly indicating a time out situation
> 3) When we want to increase the max timeout value to verify if a test case fails due to a not-enough time out value, we have to recompile & redeploy code
> I propose to create a waitForCondition function as a test utility function like the following:
> {code}
>     public interface WaitCheck {
>         public boolean Check() ;
>     }
>     public boolean waitForCondition(int timeOutInMilliSeconds, int checkIntervalInMilliSeconds, WaitCheck s)
>             throws InterruptedException {
>         int multiplier = 1;
>         String multiplierProp = System.getProperty("extremeWaitMultiplier");
>         if(multiplierProp != null) {
>             multiplier = Integer.parseInt(multiplierProp);
>             if(multiplier < 1) {
>                 LOG.warn(String.format("Invalid extremeWaitMultiplier property value:%s. is ignored.", multiplierProp));
>                 multiplier = 1;
>             }
>         }
>         int timeElapsed = 0;
>         while(timeElapsed < timeOutInMilliSeconds * multiplier) {
>             if(s.Check()) {
>                 return true;
>             }
>             Thread.sleep(checkIntervalInMilliSeconds);
>             timeElapsed += checkIntervalInMilliSeconds;
>         }
>         assertTrue("WaitForCondition failed due to time out(" + timeOutInMilliSeconds + " milliseconds expired)",
>                 false);
>         return false;
>     }
> {code}
> By doing the above way, there are several advantages:
> 1) Clearly report time out error when such situation happens
> 2) Use System property extremeWaitMultiplier to increase max time out dynamically for a quick verification
> 3) Standardize current wait situations
> Pleas let me know what your thoughts on this.
> Thanks,
> -Jeffrey

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira