[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed

[GitBox <gi...@apache.org>]

DaanHoogland commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433688523



##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4564,10 +4558,6 @@ private void scanStalledVMInTransitionStateOnDisconnectedHosts() {
             } catch (final Throwable e) {
             }

Review comment:
       should we add some trace logging here?

##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4599,10 +4584,6 @@ private void scanStalledVMInTransitionStateOnDisconnectedHosts() {
             } catch (final Throwable e) {
             }

Review comment:
       let's log before ignoring

##########
File path: framework/db/src/main/java/com/cloud/utils/db/DbUtil.java
##########
@@ -16,6 +16,8 @@
 // under the License.
 package com.cloud.utils.db;
 
+import static com.cloud.utils.AutoCloseableUtil.closeAutoCloseable;
+

Review comment:
       unintended change?

##########
File path: framework/db/src/main/java/com/cloud/utils/db/TransactionMBeanImpl.java
##########
@@ -45,14 +48,15 @@ public void removeTransaction(TransactionLegacy txn) {
 
     @Override
     public int getTransactionCount() {
-        return _txns.size();
+        return totalTransactionCount.intValue();
     }
 
     @Override
     public int[] getActiveTransactionCount() {
-        int[] count = new int[2];
+        int[] count = new int[3];
         count[0] = 0;
         count[1] = 0;
+        count[2] = _txns.size();

Review comment:
       change to `getTransactionCount()`?

##########
File path: framework/db/src/main/java/com/cloud/utils/db/TransactionMBeanImpl.java
##########
@@ -45,14 +48,15 @@ public void removeTransaction(TransactionLegacy txn) {
 

Review comment:
       I would expect a decrement-call in the `removeTransaction(..)`




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

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