OADP-1067: Add queuing for VSB and VSR #190

eemcmullan · 2023-02-17T17:00:34Z

dataMoverImageFqin: 'quay.io/emcmulla/data-mover:batching'

openshift-ci-robot · 2023-02-17T17:01:40Z

@eemcmullan: This pull request references OADP-1067 which is a valid jira issue.

In response to this:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-02-17T20:09:04Z

@eemcmullan: This pull request references OADP-1067 which is a valid jira issue.

In response to this:

dataMoverImageFqin: 'quay.io/emcmulla/data-mover:batching

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

openshift-ci-robot · 2023-02-17T20:18:09Z

@eemcmullan: This pull request references OADP-1067 which is a valid jira issue.

In response to this:

dataMoverImageFqin: 'quay.io/emcmulla/data-mover:batching'

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

controllers/volumesnapshotbackup_controller.go

kaovilai · 2023-03-06T18:03:33Z

Please add a logic to decrement processingVSBs
here
https://github.com/migtools/volume-snapshot-mover/blob/master/controllers/volumesnapshotbackup_controller.go#L123-L136

where if deletionTimestamp is not nil, and if InProgress..
cancel/delete volsync replicationsource/replicationdestination
and only then processingVSBs are decremented.

Otherwise update status to say deletion waiting for volsync to process etc.

shubham-pampattiwar · 2023-03-09T04:11:41Z

@eemcmullan Please add the suggested changes and resolve the conflicts on this PR

eemcmullan · 2023-03-14T15:25:55Z

@kaovilai @shubham-pampattiwar I think when we discussed the batching originally, we wanted to add to processing VSBs once volsync completed. If we wait until VSBs and its resources are cleaned-up/deleted, it will take longer.

shubham-pampattiwar · 2023-03-14T15:33:50Z

@eemcmullan But that would mean the VSB/VSR controller is still processing instance of VSB/VSR in reality but the batchingStatus would show things differently.

kaovilai · 2023-03-14T15:39:13Z

The concern here is that I got into a scenario where I deleted VSB before volsync completes.. which means nothing ever got moved to "Completed" which means the processingVSBs-- will never be called and the vsm deployment hangs in the state of "full queue" forever.

kaovilai · 2023-03-14T15:42:14Z

VSM controller should account for and handle unexpected/force deletions of VSBs and account for them in the processingVSBs variable.

Alternatively.. on a recurring basis check number of existing in-progress VSBs and use that to reset the processingVSBs variable.

Relying on status of available (not yet deleted) VSBs moving to complete for decrementing may not be reliable.

eemcmullan · 2023-03-14T15:51:32Z

@kaovilai gotcha. Will update to fix shortly

…mover into batching

api/v1alpha1/volumesnapshotbackup_types.go

api/v1alpha1/volumesnapshotrestore_types.go

controllers/volumesnapshotbackup_controller.go

controllers/common.go

kaovilai

/lgtm

kaovilai · 2023-03-17T15:51:21Z

controllers/volumesnapshotbackup_controller.go

 	if !vsb.DeletionTimestamp.IsZero() {
+		// if batchingStatus is completed then processingVSBs has already been decremented
+		if vsb.Status.BatchingStatus != "" && vsb.Status.BatchingStatus != volsnapmoverv1alpha1.SnapMoverBackupBatchingCompleted {
+			processingVSBs--
+		}


Hopefully we can do similar thing for VSR

that's the plan

shubham-pampattiwar · 2023-03-17T16:54:16Z

controllers/volumesnapshotbackup_controller.go

 	if !vsb.DeletionTimestamp.IsZero() {
+		// if batchingStatus is completed then processingVSBs has already been decremented


Here the comment does not match with the condition, are we checking for BatchingStatus == completed or BatchingStatus != completed ?

So processingVSBs already gets decremented once SnapMoverBackupBatchingCompleted. Therefore we would not want to decrement again if a VSB has that status but then starts to be deleted, because then it would be decremented twice for a single VSB

controllers/volumesnapshotrestore_controller.go

openshift-ci · 2023-03-20T19:35:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eemcmullan, kaovilai, shubham-pampattiwar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [eemcmullan,kaovilai,shubham-pampattiwar]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

eemcmullan requested a review from shubham-pampattiwar February 17, 2023 17:00

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 17, 2023

eemcmullan changed the title ~~Add batching for VSB and VSR~~ OADP-1067: Add batching for VSB and VSR Feb 17, 2023

openshift-ci-robot added the jira/valid-reference label Feb 17, 2023

eemcmullan self-assigned this Feb 23, 2023

eemcmullan marked this pull request as ready for review February 28, 2023 21:57

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 28, 2023

openshift-ci bot requested a review from sseago February 28, 2023 21:57

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 2, 2023

kaovilai requested changes Mar 3, 2023

View reviewed changes

controllers/volumesnapshotbackup_controller.go Outdated Show resolved Hide resolved

openshift-ci bot assigned kaovilai Mar 3, 2023

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 6, 2023

eemcmullan requested a review from kaovilai March 6, 2023 15:20

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 6, 2023

eemcmullan added 2 commits March 14, 2023 11:09

add batching for VSB and VSR

4b185b2

move batch status update to volsync update

4112e88

eemcmullan force-pushed the batching branch from 9367d1e to 4112e88 Compare March 14, 2023 15:20

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 14, 2023

eemcmullan unassigned kaovilai Mar 14, 2023

eemcmullan added 2 commits March 15, 2023 11:37

Merge branch 'master' of https://github.com/konveyor/volume-snapshot-…

35cc875

…mover into batching

dec processingvsbs on vsb deletion

6c238ca

openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2023

merge

2faa3fe

openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 15, 2023

shubham-pampattiwar reviewed Mar 16, 2023

View reviewed changes

api/v1alpha1/volumesnapshotbackup_types.go Outdated Show resolved Hide resolved

shubham-pampattiwar reviewed Mar 16, 2023

View reviewed changes

api/v1alpha1/volumesnapshotrestore_types.go Outdated Show resolved Hide resolved

shubham-pampattiwar reviewed Mar 16, 2023

View reviewed changes

controllers/volumesnapshotbackup_controller.go Outdated Show resolved Hide resolved

shubham-pampattiwar reviewed Mar 16, 2023

View reviewed changes

controllers/volumesnapshotbackup_controller.go Outdated Show resolved Hide resolved

eemcmullan commented Mar 16, 2023

View reviewed changes

controllers/common.go Show resolved Hide resolved

eemcmullan added 2 commits March 16, 2023 15:01

remove unused api types

cae63f9

add VSR queue func

9b23469

eemcmullan requested a review from shubham-pampattiwar March 16, 2023 20:19

eemcmullan changed the title ~~OADP-1067: Add batching for VSB and VSR~~ OADP-1067: Add queuing for VSB and VSR Mar 16, 2023

kaovilai approved these changes Mar 17, 2023

View reviewed changes

openshift-ci bot assigned kaovilai Mar 17, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2023

kaovilai reviewed Mar 17, 2023

View reviewed changes

shubham-pampattiwar reviewed Mar 17, 2023

View reviewed changes

controllers/volumesnapshotrestore_controller.go Outdated Show resolved Hide resolved

do not cover error

4dd79be

openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2023

eemcmullan requested a review from shubham-pampattiwar March 20, 2023 14:54

fix comment

d7cc7ad

shubham-pampattiwar approved these changes Mar 20, 2023

View reviewed changes

openshift-ci bot assigned shubham-pampattiwar Mar 20, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 20, 2023

eemcmullan merged commit 4a6daa0 into migtools:master Mar 20, 2023
6 checks passed

eemcmullan deleted the batching branch April 25, 2023 18:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OADP-1067: Add queuing for VSB and VSR #190

OADP-1067: Add queuing for VSB and VSR #190

eemcmullan commented Feb 17, 2023 •

edited

openshift-ci-robot commented Feb 17, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Feb 17, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Feb 17, 2023 •

edited by openshift-ci bot

kaovilai commented Mar 6, 2023 •

edited

shubham-pampattiwar commented Mar 9, 2023 •

edited

eemcmullan commented Mar 14, 2023

shubham-pampattiwar commented Mar 14, 2023

kaovilai commented Mar 14, 2023

kaovilai commented Mar 14, 2023

eemcmullan commented Mar 14, 2023

kaovilai left a comment

kaovilai Mar 17, 2023

eemcmullan Mar 17, 2023

shubham-pampattiwar Mar 17, 2023

eemcmullan Mar 17, 2023

openshift-ci bot commented Mar 20, 2023

		if !vsb.DeletionTimestamp.IsZero() {
		// if batchingStatus is completed then processingVSBs has already been decremented

OADP-1067: Add queuing for VSB and VSR #190

OADP-1067: Add queuing for VSB and VSR #190

Conversation

eemcmullan commented Feb 17, 2023 • edited

openshift-ci-robot commented Feb 17, 2023 • edited by openshift-ci bot

openshift-ci-robot commented Feb 17, 2023 • edited by openshift-ci bot

openshift-ci-robot commented Feb 17, 2023 • edited by openshift-ci bot

kaovilai commented Mar 6, 2023 • edited

shubham-pampattiwar commented Mar 9, 2023 • edited

eemcmullan commented Mar 14, 2023

shubham-pampattiwar commented Mar 14, 2023

kaovilai commented Mar 14, 2023

kaovilai commented Mar 14, 2023

eemcmullan commented Mar 14, 2023

kaovilai left a comment

Choose a reason for hiding this comment

kaovilai Mar 17, 2023

Choose a reason for hiding this comment

eemcmullan Mar 17, 2023

Choose a reason for hiding this comment

shubham-pampattiwar Mar 17, 2023

Choose a reason for hiding this comment

eemcmullan Mar 17, 2023

Choose a reason for hiding this comment

openshift-ci bot commented Mar 20, 2023

eemcmullan commented Feb 17, 2023 •

edited

openshift-ci-robot commented Feb 17, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Feb 17, 2023 •

edited by openshift-ci bot

openshift-ci-robot commented Feb 17, 2023 •

edited by openshift-ci bot

kaovilai commented Mar 6, 2023 •

edited

shubham-pampattiwar commented Mar 9, 2023 •

edited