From 1ed1beaaa4466fb55d7dba3e029b583b835b8aaf Mon Sep 17 00:00:00 2001 From: Jim Hahn Date: Fri, 29 Jun 2018 10:54:00 -0400 Subject: 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 --- .../sql/1811-distributedlocking.downgrade.sql | 19 +++++++++++++ .../sql/1811-distributedlocking.upgrade.sql | 23 ++++++++++++++++ .../locking/DistributedLockingFeature.java | 4 +-- .../policy/distributed/locking/TargetLock.java | 32 ++++++++++------------ .../policy/distributed/locking/TargetLockTest.java | 16 +++++------ 5 files changed, 65 insertions(+), 29 deletions(-) create mode 100644 feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.downgrade.sql create mode 100644 feature-distributed-locking/src/main/feature/db/pooling/sql/1811-distributedlocking.upgrade.sql (limited to 'feature-distributed-locking') 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(); -- cgit 1.2.3-korg