diff options
author | Jim Hahn <jrh3@att.com> | 2018-06-29 10:54:00 -0400 |
---|---|---|
committer | Jim Hahn <jrh3@att.com> | 2018-06-29 12:25:04 -0400 |
commit | 1ed1beaaa4466fb55d7dba3e029b583b835b8aaf (patch) | |
tree | 0e5e1c3ff6cd5ca78ab76b491d21676c0b8ff111 | |
parent | ee2313141a40222a83d6bf39a15c3f9d4c3a239e (diff) |
Use DB time instead of jvm time
Modified distributed locking code to use the DB time (i.e., "now()") when
determining expiration times of locks. This will eliminate concerns that
may arise from different timestamps on different JVMs. As part of the
change, the expirationTime column was changed from BIGINT to TIMESTAMP.
Rename 1810 sql scripts to 1811, to match release date.
Change-Id: Ibfb15742f447133b001e4340027657ac202864a6
Issue-ID: POLICY-872
Signed-off-by: Jim Hahn <jrh3@att.com>
5 files changed, 65 insertions, 29 deletions
diff --git a/feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.downgrade.sql b/feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.downgrade.sql new file mode 100644 index 00000000..33f6154d --- /dev/null +++ b/feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.downgrade.sql @@ -0,0 +1,19 @@ +# ============LICENSE_START======================================================= +# feature-distributed-locking +# ================================================================================ +# Copyright (C) 2018 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= + +ALTER TABLE pooling.locks modify if exists expirationTime bigint(20); diff --git a/feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.upgrade.sql b/feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.upgrade.sql new file mode 100644 index 00000000..7862da9f --- /dev/null +++ b/feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.upgrade.sql @@ -0,0 +1,23 @@ +# ============LICENSE_START======================================================= +# feature-distributed-locking +# ================================================================================ +# Copyright (C) 2018 AT&T Intellectual Property. All rights reserved. +# ================================================================================ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# ============LICENSE_END========================================================= + + set foreign_key_checks=0; + + ALTER TABLE pooling.locks modify if exists expirationTime timestamp default 0; + + set foreign_key_checks=1; diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java index 03872e1e..88035ca7 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/DistributedLockingFeature.java @@ -152,11 +152,11 @@ public class DistributedLockingFeature implements PolicyEngineFeatureAPI, Policy private void cleanLockTable() { try (Connection conn = dataSource.getConnection(); - PreparedStatement statement = conn.prepareStatement("DELETE FROM pooling.locks WHERE host = ? OR expirationTime < ?"); + PreparedStatement statement = conn.prepareStatement( + "DELETE FROM pooling.locks WHERE host = ? OR expirationTime < now()"); ){ statement.setString(1, uuid.toString()); - statement.setLong(2, System.currentTimeMillis()); statement.executeUpdate(); } catch (SQLException e) { diff --git a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java index 0853f125..fe6f2fe0 100644 --- a/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java +++ b/feature-distributed-locking/src/main/java/org/onap/policy/distributed/locking/TargetLock.java @@ -25,7 +25,6 @@ import java.sql.ResultSet; import java.sql.SQLException; import java.util.UUID; import org.apache.commons.dbcp2.BasicDataSource; -import java.util.concurrent.TimeUnit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -72,7 +71,7 @@ public class TargetLock { */ public boolean lock(int holdSec) { - return grabLock(TimeUnit.SECONDS.toMillis(holdSec)); + return grabLock(holdSec); } /** @@ -86,27 +85,27 @@ public class TargetLock { * "Grabs" lock by attempting to insert a new record in the db. * If the insert fails due to duplicate key error resource is already locked * so we call secondGrab. - * @param holdMs the amount of time, in milliseconds, that the lock should be held + * @param holdSec the amount of time, in seconds, that the lock should be held */ - private boolean grabLock(long holdMs) { + private boolean grabLock(int holdSec) { // try to insert a record into the table(thereby grabbing the lock) try (Connection conn = dataSource.getConnection(); PreparedStatement statement = conn.prepareStatement( - "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)")) { + "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, timestampadd(second, ?, now()))")) { int i = 1; statement.setString(i++, this.resourceId); statement.setString(i++, this.uuid.toString()); statement.setString(i++, this.owner); - statement.setLong(i++, System.currentTimeMillis() + holdMs); + statement.setInt(i++, holdSec); statement.executeUpdate(); } catch (SQLException e) { logger.error("error in TargetLock.grabLock()", e); - return secondGrab(holdMs); + return secondGrab(holdSec); } return true; @@ -115,25 +114,24 @@ public class TargetLock { /** * A second attempt at grabbing a lock. It first attempts to update the lock in case it is expired. * If that fails, it attempts to insert a new record again - * @param holdMs the amount of time, in milliseconds, that the lock should be held + * @param holdSec the amount of time, in seconds, that the lock should be held */ - private boolean secondGrab(long holdMs) { + private boolean secondGrab(int holdSec) { try (Connection conn = dataSource.getConnection(); PreparedStatement updateStatement = conn.prepareStatement( - "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = ? WHERE resourceId = ? AND (owner = ? OR expirationTime <= ?)"); + "UPDATE pooling.locks SET host = ?, owner = ?, expirationTime = timestampadd(second, ?, now()) WHERE resourceId = ? AND (owner = ? OR expirationTime < now())"); PreparedStatement insertStatement = conn.prepareStatement( - "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, ?)");) { + "INSERT INTO pooling.locks (resourceId, host, owner, expirationTime) values (?, ?, ?, timestampadd(second, ?, now()))");) { int i = 1; updateStatement.setString(i++, this.uuid.toString()); updateStatement.setString(i++, this.owner); - updateStatement.setLong(i++, System.currentTimeMillis() + holdMs); + updateStatement.setInt(i++, holdSec); updateStatement.setString(i++, this.resourceId); updateStatement.setString(i++, this.owner); - updateStatement.setLong(i++, System.currentTimeMillis()); // The lock was expired and we grabbed it. // return true @@ -148,7 +146,7 @@ public class TargetLock { insertStatement.setString(i++, this.resourceId); insertStatement.setString(i++, this.uuid.toString()); insertStatement.setString(i++, this.owner); - insertStatement.setLong(i++, System.currentTimeMillis() + holdMs); + insertStatement.setInt(i++, holdSec); // If our insert returns 1 we successfully grabbed the lock return (insertStatement.executeUpdate() == 1); @@ -191,12 +189,11 @@ public class TargetLock { try (Connection conn = dataSource.getConnection(); PreparedStatement selectStatement = conn.prepareStatement( - "SELECT * FROM pooling.locks WHERE resourceId = ? AND host = ? AND owner= ? AND expirationTime >= ?")) { + "SELECT * FROM pooling.locks WHERE resourceId = ? AND host = ? AND owner= ? AND expirationTime >= now()")) { selectStatement.setString(1, this.resourceId); selectStatement.setString(2, this.uuid.toString()); selectStatement.setString(3, this.owner); - selectStatement.setLong(4, System.currentTimeMillis()); try (ResultSet result = selectStatement.executeQuery()) { // This will return true if the @@ -221,10 +218,9 @@ public class TargetLock { try (Connection conn = dataSource.getConnection(); PreparedStatement selectStatement = conn - .prepareStatement("SELECT * FROM pooling.locks WHERE resourceId = ? AND expirationTime >= ?")) { + .prepareStatement("SELECT * FROM pooling.locks WHERE resourceId = ? AND expirationTime >= now()")) { selectStatement.setString(1, this.resourceId); - selectStatement.setLong(2, System.currentTimeMillis()); try (ResultSet result = selectStatement.executeQuery()) { // This will return true if the // query returned at least one row diff --git a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java index a4c292c1..c1b46d67 100644 --- a/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java +++ b/feature-distributed-locking/src/test/java/org/onap/policy/distributed/locking/TargetLockTest.java @@ -80,10 +80,9 @@ public class TargetLockTest { assertEquals(OperResult.OPER_ACCEPTED, distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC)); //attempt to grab expiredLock - try (PreparedStatement updateStatement = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = ? WHERE resourceId = ?");) + try (PreparedStatement updateStatement = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = timestampadd(second, -1, now()) WHERE resourceId = ?");) { - updateStatement.setLong(1, System.currentTimeMillis() - 1000); - updateStatement.setString(2, "resource1"); + updateStatement.setString(1, "resource1"); updateStatement.executeUpdate(); } catch (SQLException e) { @@ -104,8 +103,8 @@ public class TargetLockTest { distLockFeat.beforeLock("resource1", "owner1", MAX_AGE_SEC); //force lock to expire - try (PreparedStatement lockExpire = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = ?");) { - lockExpire.setLong(1, System.currentTimeMillis() - MAX_AGE_SEC - 1); + try (PreparedStatement lockExpire = conn.prepareStatement("UPDATE pooling.locks SET expirationTime = timestampadd(second, -?, now())");) { + lockExpire.setInt(1, MAX_AGE_SEC + 1); lockExpire.executeUpdate(); } @@ -139,9 +138,8 @@ public class TargetLockTest { // isActive on expiredLock try (PreparedStatement updateStatement = conn - .prepareStatement("UPDATE pooling.locks SET expirationTime = ? WHERE resourceId = ?");) { - updateStatement.setLong(1, System.currentTimeMillis() - 5000); - updateStatement.setString(2, "resource1"); + .prepareStatement("UPDATE pooling.locks SET expirationTime = timestampadd(second, -5, now()) WHERE resourceId = ?");) { + updateStatement.setString(1, "resource1"); updateStatement.executeUpdate(); } catch (SQLException e) { @@ -183,7 +181,7 @@ public class TargetLockTest { } private static void createTable() { - String createString = "create table if not exists pooling.locks (resourceId VARCHAR(128), host VARCHAR(128), owner VARCHAR(128), expirationTime BIGINT, PRIMARY KEY (resourceId))"; + String createString = "create table if not exists pooling.locks (resourceId VARCHAR(128), host VARCHAR(128), owner VARCHAR(128), expirationTime TIMESTAMP DEFAULT 0, PRIMARY KEY (resourceId))"; try (PreparedStatement createStmt = conn.prepareStatement(createString);) { createStmt.executeUpdate(); |