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
