After further discussion, based on GC variability +1 for what Dan said. Thanks, Mark
> On Jul 11, 2018, at 12:53 PM, Jacob Barrett <jbarr...@pivotal.io> wrote: > > +1 what Dan said. > >> On Jul 11, 2018, at 11:16 AM, Dan Smith <dsm...@pivotal.io> wrote: >> >> Well, some of these tests are waiting for members to startup, etc. If the >> machine they are running on is slow, that could take more than a minute. >> >> The point here is that these are not tests of how long it takes do a geode >> operation. That's what performance tests are for. These tests just have an >> atMost because we want them to fail, rather than hang, if the assertion is >> never satisfied. Because these tests should always pass in a variety of >> environments, we should set atMost to be something really large. >> >> Performance tests that have assertions about timing need to run on known >> hardware, and generally need to assert things like 99.9% the response time >> is within this window. That's not what this test suite is. >> >> -Dan >> >>> On Wed, Jul 11, 2018 at 10:05 AM, Udo Kohlmeyer <u...@apache.org> wrote: >>> >>> Hi there Dan, >>> >>> Whilst 5min seems to be a viable option, I believe that knowing how long >>> an operation should take and reacting if it doesn't complete in that time >>> is better than waiting a standard amount of time. I like the faster >>> feedback option, rather than the standard timeout across the board. Now, >>> that said, crazy timeouts like 1-10s are maybe a little low. >>> >>> Maybe 1min rather than 5min? With exception to disk recovery tests, which >>> feasibly could take more than 1min. I cannot think of a single operation in >>> Geode, that should realistically should take more than 60s. >>> >>> --Udo >>> >>> >>> >>>> On 7/11/18 09:07, Dan Smith wrote: >>>> >>>> Hi all, >>>> >>>> We have a bunch of tests that are using awaitility. It seems like every >>>> tests is picking some random number of it's timeout, usually in the range >>>> of 10-60 seconds. >>>> >>>> I'd like to change all of our tests to use a standard timeout that is much >>>> higher, to avoid worrying about whether our timeouts are to low. So I >>>> propose introducing our own GeodeAwaitility class that sets a timeout of 5 >>>> minutes and replacing all of our usage with that. >>>> >>>> -Dan >>>> >>>> >>>