Setting snapshot removed on timeout by Spaceman1984 · Pull Request #4425 · apache/cloudstack

Spaceman1984 · 2020-10-26T07:51:33Z

Description

When creating a snapshot times-out on the management server, the record in snapshot_store_ref table is deleted.

By setting the snapshot to "removed" on timeout, the record is not deleted.

Fixes: #3559

Types of changes

Breaking change (fix or feature that would cause existing functionality to change)
New feature (non-breaking change which adds functionality)
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)

Screenshots (if appropriate):

How Has This Been Tested?

Set the global variable: backup.snapshot.wait to 2 seconds.
Create a volume snapshot.
Check the snapshots table for a snapshot record and check for a corresponding record in snapshot_store_ref table.

Spaceman1984 · 2020-10-26T07:51:51Z

@blueorangutan package

blueorangutan · 2020-10-26T07:52:28Z

@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-10-26T08:24:21Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2276

Spaceman1984 · 2020-10-26T08:55:00Z

@blueorangutan test

blueorangutan · 2020-10-26T08:56:21Z

@Spaceman1984 a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

GabrielBrascher

Code LGTM

blueorangutan · 2020-10-26T19:55:14Z

Trillian test result (tid-3075)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 37527 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4425-t3075-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_vpc_redundant.py
Smoke tests completed. 84 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	284.61	test_kubernetes_clusters.py

davidjumani

LGTM

davidjumani

Discussed an issue with @Spaceman1984
Might be unrelated, Waiting for his response

DaanHoogland · 2020-10-27T13:43:36Z

Discussed an issue with @Spaceman1984
Might be unrelated, Waiting for his response

so this is on hold, @davidjumani ?

Spaceman1984 · 2020-10-27T14:00:25Z

I'll convert to draft @DaanHoogland

Spaceman1984 · 2020-10-28T06:52:02Z

Snapshots are being deleted from the database when using the "Async" flag on creation and it times out. - This is happening in a separate process - I suspect the snapshots are being marked for garbage collection in the async process. I'll need to look into this a bit further.

yadvr · 2020-10-28T10:39:31Z

@Spaceman1984 can you share the latest update on this, is this ready for review/testing - or you still investigating?

Spaceman1984 · 2020-10-28T11:10:04Z

Still investigating @rhtyd

yadvr · 2020-11-02T08:39:08Z

@Spaceman1984 any update on this?

Spaceman1984 · 2020-11-02T10:41:40Z

No progress yet @rhtyd , I have been preoccupied with some other work-related stuff, I was only able to start looking at this again today.

Spaceman1984 · 2020-11-03T07:32:46Z

In the storage garbage collection process snapshots in an error state are deleted where the "removed" field is null.

[StorageManagerImpl.java]

// remove snapshots in Error state
List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);
for (SnapshotVO snapshotVO : snapshots) {
    try {
        List<SnapshotDataStoreVO> storeRefs = _snapshotStoreDao.findBySnapshotId(snapshotVO.getId());
        for (SnapshotDataStoreVO ref : storeRefs) {
            _snapshotStoreDao.expunge(ref.getId());
        }
        _snapshotDao.expunge(snapshotVO.getId());
    } catch (Exception e) {
        s_logger.warn("Unable to destroy snapshot " + snapshotVO.getUuid(), e);
    }
}

This line only looks at where records are not marked as removed:
List<SnapshotVO> snapshots = _snapshotDao.listAllByStatus(Snapshot.State.Error);

I can do a few things here, I can set the removed field to the current timestamp on error which should prevent the field from being deleted or I can create a different TimedOut state so these records are also not cleaned out by the garbage collection process.

yadvr · 2020-11-06T09:13:59Z

@Spaceman1984 please update if this is ready for review?

…tion

yadvr · 2020-11-18T23:26:58Z

@blueorangutan package

@sureshanaparti @borisstoyanov any final comments

blueorangutan · 2020-11-18T23:28:54Z

@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-11-19T00:29:48Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2405

DaanHoogland · 2020-11-19T08:46:55Z

@blueorangutan test matrix

blueorangutan · 2020-11-19T08:47:55Z

@DaanHoogland a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

Spaceman1984 · 2020-11-19T09:14:09Z

@blueorangutan package

blueorangutan · 2020-11-19T09:14:56Z

@Spaceman1984 a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress.

blueorangutan · 2020-11-19T09:51:11Z

Packaging result: ✔centos7 ✔centos8 ✔debian. JID-2408

Spaceman1984 · 2020-11-19T09:56:14Z

@blueorangutan test matrix

blueorangutan · 2020-11-19T09:57:01Z

@Spaceman1984 a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

sureshanaparti

changes LGTM

blueorangutan · 2020-11-19T19:07:33Z

Trillian test result (tid-3209)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 31247 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4425-t3209-xenserver-71.zip
Intermittent failure detected: /marvin/tests/smoke/test_scale_vm.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_01_scale_vm	`Failure`	9.34	test_scale_vm.py

blueorangutan · 2020-11-19T23:06:20Z

Trillian test result (tid-3216)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 38018 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4425-t3216-vmware-65u2.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Smoke tests completed. 85 look OK, 1 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	777.31	test_kubernetes_clusters.py

blueorangutan · 2020-11-20T01:26:23Z

Trillian test result (tid-3217)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 41875 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4425-t3217-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_certauthority_root.py
Intermittent failure detected: /marvin/tests/smoke/test_internal_lb.py
Intermittent failure detected: /marvin/tests/smoke/test_iso.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_supported_versions.py
Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
Intermittent failure detected: /marvin/tests/smoke/test_loadbalance.py
Intermittent failure detected: /marvin/tests/smoke/test_metrics_api.py
Intermittent failure detected: /marvin/tests/smoke/test_multipleips_per_nic.py
Intermittent failure detected: /marvin/tests/smoke/test_nested_virtualization.py
Intermittent failure detected: /marvin/tests/smoke/test_network_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_network.py
Intermittent failure detected: /marvin/tests/smoke/test_password_server.py
Intermittent failure detected: /marvin/tests/smoke/test_portforwardingrules.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Intermittent failure detected: /marvin/tests/smoke/test_projects.py
Intermittent failure detected: /marvin/tests/smoke/test_reset_vm_on_reboot.py
Smoke tests completed. 70 look OK, 16 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File
test_provision_certificate	`Error`	121.05	test_certauthority_root.py
ContextSuite context=TestInternalLb>:setup	`Error`	0.00	test_internal_lb.py
test_01_1_create_iso_with_checksum_sha1_negative	`Error`	66.51	test_iso.py
test_01_create_iso_with_checksum_sha1	`Error`	65.42	test_iso.py
test_01_create_iso_with_checksum_sha1	`Error`	66.50	test_iso.py
test_02_1_create_iso_with_checksum_sha256_negative	`Error`	66.50	test_iso.py
test_02_create_iso_with_checksum_sha256	`Error`	65.42	test_iso.py
test_02_create_iso_with_checksum_sha256	`Error`	66.51	test_iso.py
test_03_1_create_iso_with_checksum_md5_negative	`Error`	66.49	test_iso.py
test_03_create_iso_with_checksum_md5	`Error`	65.41	test_iso.py
test_03_create_iso_with_checksum_md5	`Error`	66.50	test_iso.py
test_04_create_iso_with_no_checksum	`Error`	65.42	test_iso.py
test_04_create_iso_with_no_checksum	`Error`	66.52	test_iso.py
test_01_create_iso	`Failure`	1513.23	test_iso.py
ContextSuite context=TestISO>:setup	`Error`	3027.46	test_iso.py
test_01_deploy_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_02_invalid_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_03_deploy_and_upgrade_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_04_deploy_and_scale_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_05_delete_kubernetes_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_06_deploy_invalid_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_07_deploy_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_08_deploy_and_upgrade_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_09_delete_kubernetes_ha_cluster	`Failure`	0.01	test_kubernetes_clusters.py
test_01_add_delete_kubernetes_supported_version	`Error`	1802.56	test_kubernetes_supported_versions.py
ContextSuite context=TestListIdsParams>:setup	`Error`	0.00	test_list_ids_parameter.py
ContextSuite context=TestLoadBalance>:setup	`Error`	0.00	test_loadbalance.py
test_list_vms_metrics	`Error`	0.19	test_metrics_api.py
test_nic_secondaryip_add_remove	`Error`	0.07	test_multipleips_per_nic.py
ContextSuite context=TestNestedVirtualization>:setup	`Error`	0.00	test_nested_virtualization.py
ContextSuite context=TestNetworkACL>:setup	`Error`	0.00	test_network_acl.py
test_delete_account	`Error`	0.62	test_network.py
test_delete_network_while_vm_on_it	`Error`	0.08	test_network.py
test_deploy_vm_l2network	`Error`	0.07	test_network.py
test_l2network_restart	`Error`	1.29	test_network.py
ContextSuite context=TestPortForwarding>:setup	`Error`	3.60	test_network.py
ContextSuite context=TestPublicIP>:setup	`Error`	1.43	test_network.py
test_reboot_router	`Error`	0.07	test_network.py
test_releaseIP	`Error`	0.64	test_network.py
ContextSuite context=TestRouterRules>:setup	`Error`	0.71	test_network.py
ContextSuite context=TestIsolatedNetworksPasswdServer>:setup	`Error`	0.00	test_password_server.py
ContextSuite context=TestPortForwardingRules>:setup	`Error`	0.00	test_portforwardingrules.py
ContextSuite context=TestPrivateGwACL>:setup	`Error`	0.00	test_privategw_acl.py
test_09_project_suspend	`Error`	1.11	test_projects.py
test_10_project_activation	`Error`	1.09	test_projects.py

DaanHoogland · 2020-11-20T07:58:33Z

prety sure the kvm failures are environmental, rerunning that one
@blueorangutan test centos7 kvm-centos7

blueorangutan · 2020-11-20T08:00:05Z

@DaanHoogland a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests

davidjumani

LGTM. Snaps removed when operation timed out when async set to both true and false

yadvr · 2020-11-20T15:38:26Z

@DaanHoogland let's merge this once the test results are back we've enough LGTMs and manual tests done

blueorangutan · 2020-11-20T18:22:39Z

Trillian test result (tid-3221)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 35467 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr4425-t3221-kvm-centos7.zip
Intermittent failure detected: /marvin/tests/smoke/test_kubernetes_clusters.py
Intermittent failure detected: /marvin/tests/smoke/test_list_ids_parameter.py
Intermittent failure detected: /marvin/tests/smoke/test_privategw_acl.py
Smoke tests completed. 86 look OK, 0 have error(s)
Only failed tests results shown below:

Test	Result	Time (s)	Test File

Setting snapshot state to error on timeout

1a0734f

GabrielBrascher approved these changes Oct 26, 2020

View reviewed changes

davidjumani approved these changes Oct 27, 2020

View reviewed changes

davidjumani reviewed Oct 27, 2020

View reviewed changes

Spaceman1984 marked this pull request as draft October 27, 2020 14:00

yadvr marked this pull request as ready for review October 28, 2020 10:38

yadvr marked this pull request as draft October 28, 2020 10:39

PaulAngus added Severity:Critical Critical bug type:bug labels Oct 29, 2020

PaulAngus added this to the 4.15.0.0 milestone Oct 29, 2020

ACSGitBot added the status:wip label Nov 3, 2020

yadvr requested review from DaanHoogland and sureshanaparti November 6, 2020 09:13

Setting removed field so snapshot record is ignored by garbage collec…

9650051

…tion

Spaceman1984 marked this pull request as ready for review November 6, 2020 11:28

sureshanaparti reviewed Nov 18, 2020

View reviewed changes

Comment thread ...datamotion/src/main/java/org/apache/cloudstack/storage/motion/AncientDataMotionStrategy.java Outdated

yadvr requested a review from sureshanaparti November 18, 2020 08:47

Renamed method, moved code a few lines down

a7574ba

Spaceman1984 added 2 commits November 19, 2020 11:01

Moved remove logic

49d6ff0

Removed unused service

24033b0

DaanHoogland requested review from DaanHoogland, GabrielBrascher and davidjumani November 19, 2020 09:17

Moved removed logic - last time, promise

0b168ea

sureshanaparti approved these changes Nov 19, 2020

View reviewed changes

yadvr requested a review from harikrishna-patnala November 20, 2020 08:47

davidjumani approved these changes Nov 20, 2020

View reviewed changes

yadvr merged commit dfa09fc into apache:master Nov 20, 2020

Uh oh!

Conversation

Spaceman1984 commented Oct 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Types of changes

Screenshots (if appropriate):

How Has This Been Tested?

Uh oh!

Spaceman1984 commented Oct 26, 2020

Uh oh!

blueorangutan commented Oct 26, 2020

Uh oh!

blueorangutan commented Oct 26, 2020

Uh oh!

Spaceman1984 commented Oct 26, 2020

Uh oh!

blueorangutan commented Oct 26, 2020

Uh oh!

GabrielBrascher left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Oct 26, 2020

Uh oh!

davidjumani left a comment

Choose a reason for hiding this comment

Uh oh!

davidjumani left a comment

Choose a reason for hiding this comment

Uh oh!

DaanHoogland commented Oct 27, 2020

Uh oh!

Spaceman1984 commented Oct 27, 2020

Uh oh!

Spaceman1984 commented Oct 28, 2020

Uh oh!

yadvr commented Oct 28, 2020

Uh oh!

Spaceman1984 commented Oct 28, 2020

Uh oh!

yadvr commented Nov 2, 2020

Uh oh!

Spaceman1984 commented Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Spaceman1984 commented Nov 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yadvr commented Nov 6, 2020

Uh oh!

Uh oh!

yadvr commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 18, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

DaanHoogland commented Nov 19, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

Spaceman1984 commented Nov 19, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

Spaceman1984 commented Nov 19, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

sureshanaparti left a comment

Choose a reason for hiding this comment

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

blueorangutan commented Nov 19, 2020

Uh oh!

blueorangutan commented Nov 20, 2020

Uh oh!

Spaceman1984 commented Oct 26, 2020 •

edited

Loading

Spaceman1984 commented Nov 2, 2020 •

edited

Loading

Spaceman1984 commented Nov 3, 2020 •

edited

Loading