ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Николай Ижиков <nizhikov....@gmail.com>
Subject Re: IgniteUtils#currentTimeMillis
Date Wed, 09 Aug 2017 12:32:20 GMT
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 <vozerov@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.spb@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.dev@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 <vozerov@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.spb@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.spb@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 <yzhdanov@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.dev@gmail.com
> > >
> >
>



-- 
Nikolay Izhikov
NIzhikov.dev@gmail.com

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message