activemq-artemis icon indicating copy to clipboard operation
activemq-artemis copied to clipboard

ARTEMIS-3763 Use margin when checking time difference between broker and database

Open MM53 opened this issue 2 years ago • 11 comments

  • Add configurable time margin for checking offset between database server and broker when updating lease lock

MM53 avatar Apr 06 '22 15:04 MM53

@franz1981 recently changed the code to use UTC. Meaning the time is controlled by the broker... did you check the semantics on main / latest release for this? why is this needed?

clebertsuconic avatar Apr 06 '22 15:04 clebertsuconic

@clebertsuconic initially I started this change for version 2.20.0 but had no time to open a PR. While rebasing my changes onto the latest version I checked the changes of @franz1981. But the query causing the problem ("SELECT CURRENT_TIMESTAMP FROM ...") was not changed. This returns the time of the database not the time of the broker as far as I know.

I described the problem in the Jira-issue. In our setup this leads to a warning log message about every 5 minutes, which can't be intended.

MM53 avatar Apr 06 '22 17:04 MM53

hi @MM53 this looks similar to maxAllowableDiffFromDBTime in Classic ActiveMQ (see https://activemq.apache.org/pluggable-storage-lockers for more info): I agree that it could be useful, but dangerous as well, because if the difference specified exceed few milliseconds it can mask other problems (ie distance between broker and DBMS, lack of CPU time on both, misaligned clocks etc etc) that would prevent the feature to work as expected, anyway.

But the query causing the problem ("SELECT CURRENT_TIMESTAMP FROM ...") was not changed

you should check the query that's actually used by your driver; I don't remember by heart if it's the right one TBH.

In short, in order to have aligned clocks, the machines need to use NTP or other (better) mechanisms to be sure that broker <-> BDMS time is correctly aligned. JDBC is not meant to be used with machines too distant

franz1981 avatar Apr 13 '22 07:04 franz1981

Hi @franz1981, thanks for your reply. I will have a closer look on the query actually used by my driver and also check if we have any possibility to further improve the time synchronization on our servers. In my tests an allowed range of 100 milliseconds was enough to reduce the logged warnings significantly.

You said a difference above a few milliseconds could already be an issue. Then we probably have to change the code anyway as currently all milliseconds were removed before comparing the system time and the database time. Which allows a difference of 999 milliseconds without any warning on a nearly random basis.

MM53 avatar Apr 13 '22 08:04 MM53

You said a difference above a few milliseconds could already be an issue. Then we probably have to change the code anyway as currently all milliseconds were removed before comparing the system time and the database time. Which allows a difference of 999 milliseconds without any warning on a nearly random basis.

Yep, I forgot about it, sorry :) Yes, few seconds actually, given that we strip the milliseconds part

franz1981 avatar Apr 13 '22 08:04 franz1981

Are there any updates on this PR?

I checked our setup and there are only a few milliseconds difference between the servers on an average. Therefore I assume this change could also be interesting for others. Additionally it would set a fixed boundary for the time offset instead of relying on striped milliseconds.

MM53 avatar May 09 '22 07:05 MM53

@clebertsuconic @franz1981 what do you think about this PR? Will this change be a useful improvement?

MM53 avatar May 31 '22 09:05 MM53

Hey @franz1981 / @clebertsuconic, I'm curious what's missing for you to merge back @MM53 PR? It's something that still limit us to use ActiveMQ in our live setup. Would be great to get this PR live soon or get an idea how to improve further? Thx

hunsalz avatar Jun 20 '22 09:06 hunsalz

@clebertsuconic / @franz1981 - Any feedback appreciated?

hunsalz avatar Jun 27 '22 07:06 hunsalz

@clebertsuconic / @franz1981 - Hey guys, do you have any feedback for us or can somebody else help?

hunsalz avatar Jul 18 '22 09:07 hunsalz

@jbertram Do you have any feedback regarding this PR?

MM53 avatar Jul 18 '22 11:07 MM53

Duplicate of #4200

MM53 avatar Sep 09 '22 07:09 MM53