Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid running more than one migration concurrently #1454

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Feb 5, 2025

Reference: https://issues.redhat.com/browse/MTV-1901

📝 Description

Avoid running a migration (start/restart) if a migration is already running by disabling/hiding relevant buttons (in page header, actions menu and plan details status page).
Also avoid adding the migration CR to cover cases in which rendering takes few seconds when you use the start/restart action item and then the start/restart button at once.

🎥 Demo

Before

Screencast.from.2025-02-05.12-10-40.webm

After

Screencast.from.2025-02-05.12-06-55.webm

@sgratch sgratch added the bug Categorizes issue or PR as related to a bug. label Feb 5, 2025
@sgratch sgratch added this to the 2.8.0 milestone Feb 5, 2025
@codecov-commenter
Copy link

codecov-commenter commented Feb 5, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.51%. Comparing base (13484d0) to head (d22b40b).
Report is 248 commits behind head on main.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1454      +/-   ##
==========================================
- Coverage   36.81%   36.51%   -0.31%     
==========================================
  Files         158      156       -2     
  Lines        2548     2528      -20     
  Branches      599      605       +6     
==========================================
- Hits          938      923      -15     
+ Misses       1428     1420       -8     
- Partials      182      185       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

const buttonStartLabel = canReStart ? t('Restart migration') : t('Start migration');

const [isStartItemEnabled, setIStartItemEnabled] = useState(canStart);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setIsStartItemEnabled

<DropdownItem
value={1}
key="start"
isDisabled={!canStart || !isStartItemEnabled}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why also !canStart? u check it on the effect

Copy link
Collaborator Author

@sgratch sgratch Feb 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was added to avoid as much as possible the following scenario:

  1. Clicking on the "start migration" button (or any other way of starting the plan not via the dropdown) => plan is marked as running
  2. Clicking on the dropdown's 'start' option (since It takes few secs for the effect to re-render, button is not disabled yet).

Without this fix:
Screencast from 2025-02-12 11-36-51.webm

With this fix:
Screencast from 2025-02-12 11-40-14.webm

Reference: https://issues.redhat.com/browse/MTV-1901

Avoid running a migration (start/restart) if a migraiton is already
running by disabling/hiding buttons and avoiding adding the migration
CR.
This will cover cases in which rendering takes few seconds when you use the start/restart
action item and then the start/restart button at once.

Signed-off-by: Sharon Gratch <[email protected]>
@sgratch sgratch force-pushed the avoid-running-concurrent-migrations branch from a892663 to 49f5760 Compare February 12, 2025 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants