I would propose to either increase TX_TIMEOUT or sleep multiplier to make
test more reliable.

On Wed, Aug 9, 2017 at 3:32 PM, Николай Ижиков <nizhikov....@gmail.com>
wrote:

> Vladimir,
>
> As far as I can understand behaviour of U.currentTimeMillis() breaks
> transaction timeout test:
>
> https://issues.apache.org/jira/browse/IGNITE-5963
>
> IgniteOptimisticTxSuspendResumeTest#testTxTimeoutOnSuspend :
>
> ```
> final Transaction tx = ignite.transactions().txStart(OPTIMISTIC,
> isolation,
> TX_TIMEOUT, 0);
>
> cache.put(1, 1);
>
> Thread.sleep(TX_TIMEOUT * 2);
>
> GridTestUtils.assertThrowsWithCause(new Callable<Object>() {
>     @Override public Object call() throws Exception {
>         tx.suspend();
>
>         return null;
>     }
> }, TransactionTimeoutException.class);
> ```
> Timeout checked like that:
>
> IgniteTxAdapter#remainingTime
>
> ```
> @Override public long remainingTime() {
>     if (timeout() <= 0)
>         return 0;
>
>     long timeLeft = timeout() - (U.currentTimeMillis() - startTime());
>
>     return timeLeft <= 0 ? -1 : timeLeft;
>
> }
>
> ```
>
>
> 2017-08-09 15:26 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>:
>
> > Guys,
> >
> > Are you really suggesting change the product for newcomer needs? This is
> > not an argument for the change. Can someone explain me what is currently
> > broken from product's perspective? Yes, we can get stale values for some
> > time, we know that. Does it break something?
> >
> > On Wed, Aug 9, 2017 at 3:11 PM, Dmitry Pavlov <dpavlov....@gmail.com>
> > wrote:
> >
> > > +1 to Nikolay, this is very often question from newcomers.
> > > It is not clear that current* method may return outdated value from
> some
> > > moment in past.
> > >
> > > Nikolay, how long outdated value can be returned by method?
> > >
> > > ср, 9 авг. 2017 г. в 15:00, Николай Ижиков <nizhikov....@gmail.com>:
> > >
> > > > Vladimir,
> > > >
> > > > > There is nothing wrong with U.currentTimeMillis() at the
> > > > moment.
> > > >
> > > > I think we can't rely on the return value for time measurement.
> > > > Is it true? Is it OK for you?
> > > >
> > > > It very counterintuitive for me as newcomer.
> > > >
> > > >
> > > > 2017-08-09 14:55 GMT+03:00 Vladimir Ozerov <voze...@gridgain.com>:
> > > >
> > > > > You cannot check it with benchmarks, because behavior of this
> method
> > > will
> > > > > vary between different JVMs, OSes and hardware. It can be different
> > > even
> > > > > with the same OS depending on it's settings. Again - let's just
> avoid
> > > > > unnecessary work. There is nothing wrong with U.currentTimeMillis()
> > at
> > > > the
> > > > > moment.
> > > > >
> > > > > On Wed, Aug 9, 2017 at 2:52 PM, Dmitry Pavlov <
> dpavlov....@gmail.com
> > >
> > > > > wrote:
> > > > >
> > > > > > Vladimir, could we check it using benchmarks? Internet contains a
> > lot
> > > > of
> > > > > > articles about this issue. But do we know if it is still actual
> for
> > > new
> > > > > > VMs?
> > > > > >
> > > > > > ср, 9 авг. 2017 г. в 14:50, Dmitry Pavlov <dpavlov....@gmail.com
> >:
> > > > > >
> > > > > > > It seems System.currentTimeMillis () is now in intrinsic list.
> > This
> > > > > means
> > > > > > > on modern JVMs performance penalty will not be so significiant.
> > > > > > >
> > > > > > > Nickolay, could you please raise standalone ticket for
> > > > > > U.currentTimeMillis
> > > > > > > () ?
> > > > > > >
> > > > > > > Could you also please check if system.nanoTime /
> > > system.currentTimeMs
> > > > > can
> > > > > > > fix https://issues.apache.org/jira/browse/IGNITE-5963 When you
> > > > create
> > > > > a
> > > > > > > PR, I can start several run for Ignite Cache 6 suite to check
> if
> > > > issue
> > > > > is
> > > > > > > still reprodacible.
> > > > > > >
> > > > > > > ср, 9 авг. 2017 г. в 14:41, Yakov Zhdanov <yzhda...@apache.org
> >:
> > > > > > >
> > > > > > >> Nickolay, IgniteUtils#currentTimeMillis() is some kind of an
> > old
> > > > > > heritage.
> > > > > > >> I guess nobody remembers when this method has been
> introduced. I
> > > > agree
> > > > > > >> that
> > > > > > >> we can use System.currentTimeMillis(). I would suggest you
> file
> > a
> > > > > ticket
> > > > > > >> and replace this method calls with System.currentTimeMillis().
> > > > Sounds
> > > > > > >> good?
> > > > > > >>
> > > > > > >> As far as reliable elapsed time measurement I agree with you
> > that
> > > > > > >> nanoTime() is better here, but it is definitely not a reason
> for
> > > > > > mentioned
> > > > > > >> failure, since that test is launched in single JVM on a
> machine
> > > that
> > > > > > most
> > > > > > >> probably does not do any ntp syncs during the test to make
> > > Ignite's
> > > > > > >> timeout
> > > > > > >> machinery fail.
> > > > > > >>
> > > > > > >> Please file a ticket to switch Ignite's timeouts to nanoTime()
> > at
> > > > some
> > > > > > >> point.
> > > > > > >>
> > > > > > >> --Yakov
> > > > > > >>
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Nikolay Izhikov
> > > > nizhikov....@gmail.com
> > > >
> > >
> >
>
>
>
> --
> Nikolay Izhikov
> nizhikov....@gmail.com
>

Reply via email to