You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2020/06/01 17:00:41 UTC
[GitHub] [cloudstack] rhtyd opened a new pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
rhtyd opened a new pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121
The TransactionLegacy instances are tracked by TransactionMBeanImpl in a
concurrent hashmap. Even when TransactionLegacy instances are closed,
the hashmap keep the references and makes it hard for the closed instances
to be garbage collected. This adds code to remove the TransactionLegacy
instance from the mbean when the instance is close().
Related: https://github.com/apache/cloudstack/issues/3987
Leaks seen by MAT:
The MBean explore shows a steady increase in the leaks:
## Types of changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] Enhancement (improves an existing feature and functionality)
- [ ] Cleanup (Code refactoring and cleanup, that may add test cases)
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637824120
Packaging result: ✔centos7 ✔debian. JID-1268
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638548176
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638584212
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640200426
In view of that we only need to have knowledge of long term behaviour. I do not think this is going to be an impediment before merging given the smoke test result and our observations in the demo environment.
We need to merge against the 4.14 branch (changed the base).
Any other tests/approvals @andrijapanicsb @borisstoyanov @GabrielBrascher @svenvogel @wido @PaulAngus @weizhouapache @... ?
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-639690374
@DaanHoogland the changes are in the B&R framework which is also tested by a simulator based plugin; so veeam B&R plugin specific testing is not necessary. The background sync thread still calls the same plugin specific interface/methods, so no changes on that boundary.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637332907
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r434507415
##########
File path: engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java
##########
@@ -19,69 +19,71 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.TimeZone;
-import org.apache.cloudstack.backup.Backup;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
+import com.cloud.exception.CloudException;
import com.cloud.usage.UsageBackupVO;
import com.cloud.utils.DateUtil;
import com.cloud.utils.db.GenericDaoBase;
-import com.cloud.utils.db.QueryBuilder;
import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
-import com.cloud.utils.db.TransactionStatus;
-import com.cloud.vm.VirtualMachine;
@Component
public class UsageBackupDaoImpl extends GenericDaoBase<UsageBackupVO, Long> implements UsageBackupDao {
public static final Logger LOGGER = Logger.getLogger(UsageBackupDaoImpl.class);
- protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM cloud_usage.usage_backup WHERE " +
+ protected static final String UPDATE_DELETED = "UPDATE usage_backup SET removed = ? WHERE account_id = ? AND vm_id = ? and removed IS NULL";
+ protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM usage_backup WHERE " +
" account_id = ? AND ((removed IS NULL AND created <= ?) OR (created BETWEEN ? AND ?) OR (removed BETWEEN ? AND ?) " +
" OR ((created <= ?) AND (removed >= ?)))";
@Override
- public void updateMetrics(final VirtualMachine vm, Backup.Metric metric) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vm.getId());
- final UsageBackupVO entry = findOneBy(qb.create());
- if (entry == null) {
- return false;
- }
- entry.setSize(metric.getBackupSize());
- entry.setProtectedSize(metric.getDataSize());
- return update(entry.getId(), entry);
+ public void updateMetrics(final Long vmId, final Long size, final Long virtualSize) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
+ SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
+ sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
+ UsageBackupVO vo = findOneBy(sc);
+ if (vo != null) {
+ vo.setSize(size);
+ vo.setProtectedSize(virtualSize);
+ update(vo.getId(), vo);
}
- });
- if (!result) {
- LOGGER.trace("Failed to update backup metrics for VM ID: " + vm.getId());
+ } catch (final Exception e) {
+ LOGGER.error("Error updating backup metrics: " + e.getMessage(), e);
+ } finally {
+ txn.close();
Review comment:
can we use try-with-resource in this case?
##########
File path: engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java
##########
@@ -19,69 +19,71 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.TimeZone;
-import org.apache.cloudstack.backup.Backup;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
+import com.cloud.exception.CloudException;
import com.cloud.usage.UsageBackupVO;
import com.cloud.utils.DateUtil;
import com.cloud.utils.db.GenericDaoBase;
-import com.cloud.utils.db.QueryBuilder;
import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
-import com.cloud.utils.db.TransactionStatus;
-import com.cloud.vm.VirtualMachine;
@Component
public class UsageBackupDaoImpl extends GenericDaoBase<UsageBackupVO, Long> implements UsageBackupDao {
public static final Logger LOGGER = Logger.getLogger(UsageBackupDaoImpl.class);
- protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM cloud_usage.usage_backup WHERE " +
+ protected static final String UPDATE_DELETED = "UPDATE usage_backup SET removed = ? WHERE account_id = ? AND vm_id = ? and removed IS NULL";
+ protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM usage_backup WHERE " +
" account_id = ? AND ((removed IS NULL AND created <= ?) OR (created BETWEEN ? AND ?) OR (removed BETWEEN ? AND ?) " +
" OR ((created <= ?) AND (removed >= ?)))";
@Override
- public void updateMetrics(final VirtualMachine vm, Backup.Metric metric) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vm.getId());
- final UsageBackupVO entry = findOneBy(qb.create());
- if (entry == null) {
- return false;
- }
- entry.setSize(metric.getBackupSize());
- entry.setProtectedSize(metric.getDataSize());
- return update(entry.getId(), entry);
+ public void updateMetrics(final Long vmId, final Long size, final Long virtualSize) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
+ SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
+ sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
+ UsageBackupVO vo = findOneBy(sc);
+ if (vo != null) {
+ vo.setSize(size);
+ vo.setProtectedSize(virtualSize);
+ update(vo.getId(), vo);
}
- });
- if (!result) {
- LOGGER.trace("Failed to update backup metrics for VM ID: " + vm.getId());
+ } catch (final Exception e) {
+ LOGGER.error("Error updating backup metrics: " + e.getMessage(), e);
+ } finally {
+ txn.close();
}
}
@Override
- public void removeUsage(Long accountId, Long zoneId, Long vmId) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getAccountId(), SearchCriteria.Op.EQ, accountId);
- qb.and(qb.entity().getZoneId(), SearchCriteria.Op.EQ, zoneId);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vmId);
- final UsageBackupVO entry = findOneBy(qb.create());
- return remove(qb.create()) > 0;
+ public void removeUsage(Long accountId, Long vmId, Date eventDate) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
Review comment:
again, can we use try-with-resource
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd merged pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd merged pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637029117
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433784473
##########
File path: engine/orchestration/src/main/java/com/cloud/vm/VirtualMachineManagerImpl.java
##########
@@ -4545,14 +4545,8 @@ private void scanStalledVMInTransitionStateOnDisconnectedHosts() {
"AND i.removed IS NULL";
final List<Long> l = new ArrayList<Long>();
- TransactionLegacy txn = null;
- try {
- txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB);
-
- PreparedStatement pstmt = null;
- try {
- pstmt = txn.prepareAutoCloseStatement(sql);
-
+ try (TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.CLOUD_DB)) {
+ try (PreparedStatement pstmt = txn.prepareAutoCloseStatement(sql)) {
Review comment:
afaict these can be combined in the same try clause
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637214940
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-644526252
ping 3 - @nvazquez @harikrishna-patnala @sureshanaparti @shwstppr @andrijapanicsb @weizhouapache @svenvogel @andrijapanicsb @wido @GabrielBrascher
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433775679
##########
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:
fxied/combined.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638548326
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640295561
<b>Trillian test result (tid-1637)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38065 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4121-t1637-kvm-centos7.zip
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637812941
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433773996
##########
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:
IDE changed order.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638554910
Packaging result: ✔centos7 ✖debian. JID-1273
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637802464
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-644598220
I think we have two lgtm and succesful tests on this @rhtyd , merge?
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637703341
Packaging result: ✔centos7 ✔debian. JID-1266
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638592129
@blueorangutan test
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637689644
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637787820
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638124157
In a test env, no leaks seen after 10hrs:
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433774346
##########
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:
Removed this, not transactions would return the current size; instead of cumulative/historic size.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637889279
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637028102
Packaging result: ✔centos7 ✔debian. JID-1262
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd closed pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-639642677
Do we do specific plugin test, @rhtyd ? (B&R)
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638592360
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638555357
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-639636533
<b>Trillian test result (tid-1625)</b>
Environment: vmware-67u3 (x2), Advanced Networking with Mgmt server 7
Total time taken: 62427 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4121-t1625-vmware-67u3.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637889027
@blueorangutan test
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by 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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637216413
Packaging result: ✖centos7 ✖debian. JID-1263
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637001172
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640209506
@blueorangutan test
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-641120904
We need another review to get this merged, so we can work towards a 4.14.1.0 release with the two critical issues (a) SG package dependency fix, (b) this issue. Please help review @weizhouapache @wido @GabrielBrascher @svenvogel @kiwiflyer @nvazquez @borisstoyanov and others
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637880253
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638190048
<b>Trillian test result (tid-1602)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 43008 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4121-t1602-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Smoke tests completed. 82 look OK, 1 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
test_01_add_delete_kubernetes_supported_version | `Error` | 1807.28 | test_kubernetes_supported_versions.py
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637015898
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640204942
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-642595233
ping2 - @weizhouapache @svenvogel @andrijapanicsb @wido @GabrielBrascher @nvazquez
----------------------------------------------------------------
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
[GitHub] [cloudstack] nvazquez commented on a change in pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
nvazquez commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r434313711
##########
File path: engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java
##########
@@ -19,69 +19,71 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.TimeZone;
-import org.apache.cloudstack.backup.Backup;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
+import com.cloud.exception.CloudException;
import com.cloud.usage.UsageBackupVO;
import com.cloud.utils.DateUtil;
import com.cloud.utils.db.GenericDaoBase;
-import com.cloud.utils.db.QueryBuilder;
import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
-import com.cloud.utils.db.TransactionStatus;
-import com.cloud.vm.VirtualMachine;
@Component
public class UsageBackupDaoImpl extends GenericDaoBase<UsageBackupVO, Long> implements UsageBackupDao {
public static final Logger LOGGER = Logger.getLogger(UsageBackupDaoImpl.class);
- protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM cloud_usage.usage_backup WHERE " +
+ protected static final String UPDATE_DELETED = "UPDATE usage_backup SET removed = ? WHERE account_id = ? AND vm_id = ? and removed IS NULL";
+ protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM usage_backup WHERE " +
" account_id = ? AND ((removed IS NULL AND created <= ?) OR (created BETWEEN ? AND ?) OR (removed BETWEEN ? AND ?) " +
" OR ((created <= ?) AND (removed >= ?)))";
@Override
- public void updateMetrics(final VirtualMachine vm, Backup.Metric metric) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vm.getId());
- final UsageBackupVO entry = findOneBy(qb.create());
- if (entry == null) {
- return false;
- }
- entry.setSize(metric.getBackupSize());
- entry.setProtectedSize(metric.getDataSize());
- return update(entry.getId(), entry);
+ public void updateMetrics(final Long vmId, final Long size, final Long virtualSize) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
+ SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
+ sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
+ UsageBackupVO vo = findOneBy(sc);
+ if (vo != null) {
+ vo.setSize(size);
+ vo.setProtectedSize(virtualSize);
+ update(vo.getId(), vo);
}
- });
- if (!result) {
- LOGGER.trace("Failed to update backup metrics for VM ID: " + vm.getId());
+ } catch (final Exception e) {
+ LOGGER.error("Error updating backup metrics: " + e.getMessage(), e);
+ } finally {
+ txn.close();
Review comment:
+1 to explicitly close the transaction. The `Transaction.execute()` method is being used widely over the codebase, looks strange that closing has not been handled on it
##########
File path: server/src/main/java/org/apache/cloudstack/backup/BackupManagerImpl.java
##########
@@ -1022,31 +1018,23 @@ protected void runInContext() {
continue;
}
- // Sync backup usage metrics
final Map<VirtualMachine, Backup.Metric> metrics = backupProvider.getBackupMetrics(dataCenter.getId(), new ArrayList<>(vms));
- final GlobalLock syncBackupMetricsLock = GlobalLock.getInternLock("BackupSyncTask_metrics_zone_" + dataCenter.getId());
Review comment:
Not locking now, how are the metrics update and backup sync affected by this?
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637788087
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637787507
After this fix, basic tests show non-linear growth:
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637028468
@blueorangutan test
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637013422
On big reason why this was not seen before is Java 11 uses G1, perhaps we should move back to ParallelGC - `-XX:+UseParallelGC`
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433775397
##########
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:
Fixed.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-639164867
@blueorangutan test centos7 vmware-67u3
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637888860
Packaging result: ✔centos7 ✔debian. JID-1269
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637233713
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638555487
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r434715643
##########
File path: engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java
##########
@@ -19,69 +19,71 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.TimeZone;
-import org.apache.cloudstack.backup.Backup;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
+import com.cloud.exception.CloudException;
import com.cloud.usage.UsageBackupVO;
import com.cloud.utils.DateUtil;
import com.cloud.utils.db.GenericDaoBase;
-import com.cloud.utils.db.QueryBuilder;
import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
-import com.cloud.utils.db.TransactionStatus;
-import com.cloud.vm.VirtualMachine;
@Component
public class UsageBackupDaoImpl extends GenericDaoBase<UsageBackupVO, Long> implements UsageBackupDao {
public static final Logger LOGGER = Logger.getLogger(UsageBackupDaoImpl.class);
- protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM cloud_usage.usage_backup WHERE " +
+ protected static final String UPDATE_DELETED = "UPDATE usage_backup SET removed = ? WHERE account_id = ? AND vm_id = ? and removed IS NULL";
+ protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM usage_backup WHERE " +
" account_id = ? AND ((removed IS NULL AND created <= ?) OR (created BETWEEN ? AND ?) OR (removed BETWEEN ? AND ?) " +
" OR ((created <= ?) AND (removed >= ?)))";
@Override
- public void updateMetrics(final VirtualMachine vm, Backup.Metric metric) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vm.getId());
- final UsageBackupVO entry = findOneBy(qb.create());
- if (entry == null) {
- return false;
- }
- entry.setSize(metric.getBackupSize());
- entry.setProtectedSize(metric.getDataSize());
- return update(entry.getId(), entry);
+ public void updateMetrics(final Long vmId, final Long size, final Long virtualSize) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
+ SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
+ sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
+ UsageBackupVO vo = findOneBy(sc);
+ if (vo != null) {
+ vo.setSize(size);
+ vo.setProtectedSize(virtualSize);
+ update(vo.getId(), vo);
}
- });
- if (!result) {
- LOGGER.trace("Failed to update backup metrics for VM ID: " + vm.getId());
+ } catch (final Exception e) {
+ LOGGER.error("Error updating backup metrics: " + e.getMessage(), e);
+ } finally {
+ txn.close();
}
}
@Override
- public void removeUsage(Long accountId, Long zoneId, Long vmId) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getAccountId(), SearchCriteria.Op.EQ, accountId);
- qb.and(qb.entity().getZoneId(), SearchCriteria.Op.EQ, zoneId);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vmId);
- final UsageBackupVO entry = findOneBy(qb.create());
- return remove(qb.create()) > 0;
+ public void removeUsage(Long accountId, Long vmId, Date eventDate) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
Review comment:
I'll fix and test that tomorrow
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637346205
Packaging result: ✔centos7 ✔debian. JID-1265
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433774087
##########
File path: framework/db/src/main/java/com/cloud/utils/db/TransactionMBeanImpl.java
##########
@@ -45,14 +48,15 @@ public void removeTransaction(TransactionLegacy txn) {
Review comment:
Fixed/removed code
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637215297
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637013964
Packaging result: ✔centos7 ✔debian. JID-1261
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637428171
@blueorangutan test
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638973408
<b>Trillian test result (tid-1604)</b>
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 42562 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4121-t1604-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 83 look OK, 0 have error(s)
Only failed tests results shown below:
Test | Result | Time (s) | Test File
--- | --- | --- | ---
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-639165131
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + vmware-67u3) has been kicked to run smoke tests
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640205200
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433785025
##########
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:
it is ignored here. how is this fixed?
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637690213
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-636987317
Now let's try to see what causes DB connection pool to not return a Connection cc @DaanHoogland @nvazquez
Thread/CPU/Memory have led to no hints so far, GC and threads have a normal pattern:
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on a change in pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r434942845
##########
File path: engine/schema/src/main/java/com/cloud/usage/dao/UsageBackupDaoImpl.java
##########
@@ -19,69 +19,71 @@
import java.sql.PreparedStatement;
import java.sql.ResultSet;
+import java.sql.SQLException;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import java.util.TimeZone;
-import org.apache.cloudstack.backup.Backup;
import org.apache.log4j.Logger;
import org.springframework.stereotype.Component;
+import com.cloud.exception.CloudException;
import com.cloud.usage.UsageBackupVO;
import com.cloud.utils.DateUtil;
import com.cloud.utils.db.GenericDaoBase;
-import com.cloud.utils.db.QueryBuilder;
import com.cloud.utils.db.SearchCriteria;
-import com.cloud.utils.db.Transaction;
-import com.cloud.utils.db.TransactionCallback;
import com.cloud.utils.db.TransactionLegacy;
-import com.cloud.utils.db.TransactionStatus;
-import com.cloud.vm.VirtualMachine;
@Component
public class UsageBackupDaoImpl extends GenericDaoBase<UsageBackupVO, Long> implements UsageBackupDao {
public static final Logger LOGGER = Logger.getLogger(UsageBackupDaoImpl.class);
- protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM cloud_usage.usage_backup WHERE " +
+ protected static final String UPDATE_DELETED = "UPDATE usage_backup SET removed = ? WHERE account_id = ? AND vm_id = ? and removed IS NULL";
+ protected static final String GET_USAGE_RECORDS_BY_ACCOUNT = "SELECT id, zone_id, account_id, domain_id, vm_id, backup_offering_id, size, protected_size, created, removed FROM usage_backup WHERE " +
" account_id = ? AND ((removed IS NULL AND created <= ?) OR (created BETWEEN ? AND ?) OR (removed BETWEEN ? AND ?) " +
" OR ((created <= ?) AND (removed >= ?)))";
@Override
- public void updateMetrics(final VirtualMachine vm, Backup.Metric metric) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vm.getId());
- final UsageBackupVO entry = findOneBy(qb.create());
- if (entry == null) {
- return false;
- }
- entry.setSize(metric.getBackupSize());
- entry.setProtectedSize(metric.getDataSize());
- return update(entry.getId(), entry);
+ public void updateMetrics(final Long vmId, final Long size, final Long virtualSize) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
+ SearchCriteria<UsageBackupVO> sc = this.createSearchCriteria();
+ sc.addAnd("vmId", SearchCriteria.Op.EQ, vmId);
+ UsageBackupVO vo = findOneBy(sc);
+ if (vo != null) {
+ vo.setSize(size);
+ vo.setProtectedSize(virtualSize);
+ update(vo.getId(), vo);
}
- });
- if (!result) {
- LOGGER.trace("Failed to update backup metrics for VM ID: " + vm.getId());
+ } catch (final Exception e) {
+ LOGGER.error("Error updating backup metrics: " + e.getMessage(), e);
+ } finally {
+ txn.close();
}
}
@Override
- public void removeUsage(Long accountId, Long zoneId, Long vmId) {
- boolean result = Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallback<Boolean>() {
- @Override
- public Boolean doInTransaction(final TransactionStatus status) {
- final QueryBuilder<UsageBackupVO> qb = QueryBuilder.create(UsageBackupVO.class);
- qb.and(qb.entity().getAccountId(), SearchCriteria.Op.EQ, accountId);
- qb.and(qb.entity().getZoneId(), SearchCriteria.Op.EQ, zoneId);
- qb.and(qb.entity().getVmId(), SearchCriteria.Op.EQ, vmId);
- final UsageBackupVO entry = findOneBy(qb.create());
- return remove(qb.create()) > 0;
+ public void removeUsage(Long accountId, Long vmId, Date eventDate) {
+ TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
+ try {
Review comment:
Here we cannot as txn is used in the catch block and try-with-resource syntax doesn't allow that.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637428824
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640209680
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637332384
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637001931
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637880594
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-644601989
Yes, let's do it.
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637015315
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637812441
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638574097
Packaging result: ✖centos7 ✖debian. JID-1275
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637800624
Packaging result: ✔centos7 ✔debian. JID-1267
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-640208754
Packaging result: ✔centos7 ✔debian. JID-1298
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-637455621
I'll tidy the PR later, still chasing what triggers connections to be not returned.
----------------------------------------------------------------
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
[GitHub] [cloudstack] DaanHoogland commented on a change in pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on a change in pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#discussion_r433785166
##########
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:
?? still ignored
----------------------------------------------------------------
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
[GitHub] [cloudstack] blueorangutan commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
blueorangutan commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638590785
Packaging result: ✔centos7 ✔debian. JID-1288
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: server: fix TransactionLegacy DB connection leaks due to DB switching by B&R thread
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-638583960
@blueorangutan package
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd closed pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd closed pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121
----------------------------------------------------------------
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
[GitHub] [cloudstack] rhtyd commented on pull request #4121: [WIP] db: remove TransactionLegacy once it is closed
Posted by GitBox <gi...@apache.org>.
rhtyd commented on pull request #4121:
URL: https://github.com/apache/cloudstack/pull/4121#issuecomment-636991695
Commit that removed it - https://github.com/apache/cloudstack/commit/7b7f4cd1fd72ece80a7efecf6d841b293773f726
@blueorangutan package
----------------------------------------------------------------
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