You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2022/04/06 15:17:25 UTC

[GitHub] [activemq-artemis] MM53 opened a new pull request, #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

MM53 opened a new pull request, #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1120743257

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1090418072

   @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?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1090499090

   @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. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] franz1981 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
franz1981 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1097705087

   > 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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1241617709

   Duplicate of #4200


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] franz1981 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
franz1981 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1097669161

   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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1187122410

   @jbertram Do you have any feedback regarding this PR?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 closed pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 closed pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database
URL: https://github.com/apache/activemq-artemis/pull/4017


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on a diff in pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on code in PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#discussion_r844184567


##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -2928,6 +2928,13 @@
                </xsd:documentation>
             </xsd:annotation>
          </xsd:element>
+         <xsd:element name="data-source-time-margin" type="xsd:string" minOccurs="0" maxOccurs="1">
+            <xsd:annotation>
+               <xsd:documentation>
+                  The maximal time offset between the broker and the database in milliseconds.

Review Comment:
   I will improve the documentation as soon as I have time.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1141875638

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] hunsalz commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
hunsalz commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1186982397

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] MM53 commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
MM53 commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1097686273

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] clebertsuconic commented on a diff in pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on code in PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#discussion_r844102697


##########
artemis-server/src/main/resources/schema/artemis-configuration.xsd:
##########
@@ -2928,6 +2928,13 @@
                </xsd:documentation>
             </xsd:annotation>
          </xsd:element>
+         <xsd:element name="data-source-time-margin" type="xsd:string" minOccurs="0" maxOccurs="1">
+            <xsd:annotation>
+               <xsd:documentation>
+                  The maximal time offset between the broker and the database in milliseconds.

Review Comment:
   it would be nice to have more information here.. at least say this is for locking.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] hunsalz commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
hunsalz commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1166982311

   @clebertsuconic / @franz1981 - Any feedback appreciated?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [activemq-artemis] hunsalz commented on pull request #4017: ARTEMIS-3763 Use margin when checking time difference between broker and database

Posted by GitBox <gi...@apache.org>.
hunsalz commented on PR #4017:
URL: https://github.com/apache/activemq-artemis/pull/4017#issuecomment-1160206615

   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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org