fvaleri commented on a change in pull request #3950:
URL: https://github.com/apache/activemq-artemis/pull/3950#discussion_r808930127
##########
File path: artemis-jdbc-store/src/main/resources/journal-sql.properties
##########
@@ -75,14 +76,15 @@ create-file-table.mysql=CREATE TABLE %s(ID BIGINT NOT NULL
AUTO_INCREMENT, FILEN
append-to-file.mysql=SELECT DATA, ID FROM %s WHERE ID=? FOR UPDATE
create-journal-table.mysql=CREATE TABLE %s(id BIGINT,recordType
SMALLINT,compactCount SMALLINT,txId BIGINT,userRecordType SMALLINT,variableSize
INTEGER,record LONGBLOB,txDataSize INTEGER,txData LONGBLOB,txCheckNoRecords
INTEGER,seq BIGINT) ENGINE=InnoDB
copy-file-record-by-id.mysql=UPDATE %1$s, (SELECT DATA AS FROM_DATA FROM %1$s
WHERE id=?) SELECT_COPY SET DATA=FROM_DATA WHERE id=?
+current-timestamp.mysql=SELECT UTC_TIMESTAMP(6)
# 4 GiB
max-blob-size.mysql=4294967296
table-names-case.mysql=lower
# Oracle SQL statements
create-file-table.oracle=CREATE TABLE %s(ID NUMBER(19) GENERATED BY DEFAULT ON
NULL AS IDENTITY, FILENAME VARCHAR(255), EXTENSION VARCHAR(10), DATA BLOB,
PRIMARY KEY(ID))
create-journal-table.oracle=CREATE TABLE %s(id NUMBER(19),recordType
NUMBER(5),compactCount NUMBER(5),txId NUMBER(19),userRecordType
NUMBER(5),variableSize NUMBER(10),record BLOB,txDataSize NUMBER(10),txData
BLOB,txCheckNoRecords NUMBER(10),seq NUMBER(19))
-
+current-timestamp.oracle=SELECT SYSTIMESTAMP AT TIME ZONE 'UTC' FROM DUAL
Review comment:
Given the different behaviors and way to express these queries, I would
say we should test them on all databases setting a system TZ different from UTC.
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcLeaseLock.java
##########
@@ -147,14 +148,13 @@ private long dbCurrentTimeMillis(Connection connection)
throws SQLException {
try (ResultSet resultSet = currentDateTime.executeQuery()) {
resultSet.next();
final long endTime = stripMilliseconds(System.currentTimeMillis());
- final Timestamp currentTimestamp = resultSet.getTimestamp(1);
- final long currentTime = currentTimestamp.getTime();
- final long currentTimeMillis = stripMilliseconds(currentTime);
- if (currentTimeMillis < startTime) {
- LOGGER.warnf("[%s] %s query currentTimestamp = %s on database
should happen AFTER %s on broker", lockName, holderId, currentTimestamp, new
Timestamp(startTime));
+ final long currentTime = resultSet.getTimestamp(1,
Calendar.getInstance(TimeZone.getTimeZone("UTC"))).getTime();
+ final long currentTimeNoMillis = stripMilliseconds(currentTime);
+ if (currentTimeNoMillis < startTime) {
Review comment:
Shouldn't this method be called something like
verifyAndGetDatabaseCurrentTimestamp to better reflect the internal logic?
##########
File path:
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/jdbc/JdbcLeaseLock.java
##########
@@ -114,14 +115,14 @@ private String readableLockStatus() {
final boolean autoCommit = connection.getAutoCommit();
connection.setAutoCommit(false);
try (PreparedStatement preparedStatement =
connection.prepareStatement(this.isLocked)) {
+ final long currentTimestamp = dbCurrentTimeMillis(connection);
Review comment:
I guess in dbCurrentTimeMillis() we should select the current UTC
timestamp query based on the database type (e.g. `current-timestamp.mssql`) and
fall back to `current-timestamp` with an appropriate warning if the database is
unknown.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
