activemq-artemis
activemq-artemis copied to clipboard
ARTEMIS-3763 Use margin when checking time difference between broker and database
- Add configurable time margin for checking offset between database server and broker when updating lease lock
@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 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.
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
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.
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
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.
@clebertsuconic @franz1981 what do you think about this PR? Will this change be a useful improvement?
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
@clebertsuconic / @franz1981 - Any feedback appreciated?
@clebertsuconic / @franz1981 - Hey guys, do you have any feedback for us or can somebody else help?
@jbertram Do you have any feedback regarding this PR?
Duplicate of #4200