weizhouapache commented on code in PR #9955:
URL: https://github.com/apache/cloudstack/pull/9955#discussion_r1977239445


##########
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java:
##########
@@ -486,6 +488,39 @@ public void check() {
         }
     }
 
+    private void initDistributedLock() {
+        LOGGER.info("Setting up distributed lock table if not created.");
+        TransactionLegacy txn = TransactionLegacy.open("initDistributedLock");
+        txn.start();
+        String errorMessage = "Unable to get the database connections";
+        try {
+            Connection conn = txn.getConnection();
+            errorMessage = "Unable to create distributed_lock table in the 
'cloud' database ";
+            String sql = "CREATE TABLE IF NOT EXISTS 
`cloud`.`distributed_lock` (" +
+                         "  `name` varchar(1024) NOT NULL," +
+                         "  `thread` varchar(1024) NOT NULL," +
+                         "  `ms_id` bigint NOT NULL, `pid` int NOT NULL," +
+                         "  `created` datetime DEFAULT NULL," +
+                         "  PRIMARY KEY (`name`)," +
+                         "  UNIQUE KEY `name` (`name`)" +
+                         ") ENGINE=InnoDB DEFAULT CHARSET=utf8";
+            try (PreparedStatement pstmt = conn.prepareStatement(sql)) {
+                pstmt.execute();
+            }
+            try (PreparedStatement pstmt = conn.prepareStatement("DELETE FROM 
cloud.distributed_lock WHERE ms_id=?")) {

Review Comment:
   maybe add a debug message for it?
   
   ```suggestion
               LOGGER.info("Deleting existing distributed locks with ms_id = " 
+ ManagementServerNode.getManagementServerId());
               try (PreparedStatement pstmt = conn.prepareStatement("DELETE 
FROM cloud.distributed_lock WHERE ms_id=?")) {
   ```



##########
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java:
##########
@@ -486,6 +488,39 @@ public void check() {
         }
     }
 
+    private void initDistributedLock() {
+        LOGGER.info("Setting up distributed lock table if not created.");
+        TransactionLegacy txn = TransactionLegacy.open("initDistributedLock");
+        txn.start();
+        String errorMessage = "Unable to get the database connections";
+        try {
+            Connection conn = txn.getConnection();
+            errorMessage = "Unable to create distributed_lock table in the 
'cloud' database ";
+            String sql = "CREATE TABLE IF NOT EXISTS 
`cloud`.`distributed_lock` (" +
+                         "  `name` varchar(1024) NOT NULL," +
+                         "  `thread` varchar(1024) NOT NULL," +
+                         "  `ms_id` bigint NOT NULL, `pid` int NOT NULL," +

Review Comment:
   ```suggestion
                            "  `ms_id` bigint NOT NULL," +
                            "  `pid` int NOT NULL," +
   ```



##########
framework/db/src/main/java/com/cloud/utils/db/DbUtil.java:
##########
@@ -199,28 +200,36 @@ public static final String getTableName(Class<?> clazz) {
     public static boolean getGlobalLock(String name, int timeoutSeconds) {
         Connection conn = getConnectionForGlobalLocks(name, true);
         if (conn == null) {
-            LOGGER.error("Unable to acquire DB connection for global lock 
system");
+            LOGGER.error("Unable to acquire DB connection for distributed 
lock: " + name);
             return false;
         }
 
-        try (PreparedStatement pstmt = conn.prepareStatement("SELECT 
COALESCE(GET_LOCK(?, ?),0)");) {
-            pstmt.setString(1, name);
-            pstmt.setInt(2, timeoutSeconds);
-
-            try (ResultSet rs = pstmt.executeQuery();) {
-                if (rs != null && rs.first()) {
-                    if (rs.getInt(1) > 0) {
-                        return true;
-                    } else {
-                        if (LOGGER.isDebugEnabled())
-                            LOGGER.debug("GET_LOCK() timed out on lock : " + 
name);
-                    }
+        int remainingTime = timeoutSeconds;
+        while (remainingTime > 0) {
+            try (PreparedStatement pstmt = conn.prepareStatement(
+                    "INSERT INTO cloud.distributed_lock (name, thread, ms_id, 
pid, created) " +

Review Comment:
   I was wondering whether "INSERT INGORE INTO" is better than "ON DUPLICATE 
KEY UPDATE", and finally get an impression that "ON DUPLICATE KEY UPDATE" looks 
better



-- 
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: commits-unsubscr...@cloudstack.apache.org

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

Reply via email to