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

[CLC-427][CLC-428]: Remove Data Migrations in Progress List and Status Map Usages #424

Merged
merged 14 commits into from
Nov 7, 2023

Conversation

kutluhanmetin
Copy link
Contributor

@kutluhanmetin kutluhanmetin commented Nov 2, 2023

  • Remove Data Migrations in Progress List and Status Map Usages
  • Refactor test files
  • Add new stage "prechecks" to start command
  • Wait for migration to be in STARTED status in start command

Must(tcx.CLC().Execute(ctx, "cancel"))
})
MustValue(l.Remove(ctx, serialization.JSON(m)))
time.Sleep(1 * time.Second)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We may have a problem if these tests don't work without sleeping.

Copy link
Member

Choose a reason for hiding this comment

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

tbh, these tests are not enough anyway. I'll build the migration cluster build from the "remove in progress list" PR and try basic things.

Copy link
Member

Choose a reason for hiding this comment

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

(also I don't get why we have sleep in this test, how mocking of migration cluster is done is complex for me. )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kutluhanmetin kutluhanmetin requested a review from yuce November 3, 2023 14:05
@yuce
Copy link
Collaborator

yuce commented Nov 3, 2023

@kutluhanmetin The tests fail. Could you have a look?

@srknzl
Copy link
Member

srknzl commented Nov 3, 2023

I have tried this version with the latest backend in the PR https://github.com/hazelcast/hazelcast-enterprise/pull/6765

The result of a dmt cancel when there's no migration in progress:

image

I guess this error is not expected

@srknzl
Copy link
Member

srknzl commented Nov 3, 2023

Status command when there's no migration:

➜ hazelcast-enterprise-5.3.5-DM-SNAPSHOT-slim ./dmt -c migration.yaml status

Hazelcast Data Migration Tool v5.3.0
(c) 2023 Hazelcast, Inc.

ERROR Could not connect to the migration cluster: finding migration in progress: no rows found

@srknzl
Copy link
Member

srknzl commented Nov 3, 2023

when there was a migration and I canceled while it's in progress:

image

This worked correctly except that the start command exited without any message. We should fix that as well (not related to this PR)

@srknzl
Copy link
Member

srknzl commented Nov 6, 2023

Note: I approved because I retried all the scenarios above and all of them are fixed now.

* add new stage "prechecks" to start command
* wait for migration to be in STARTED status in start command
@kutluhanmetin kutluhanmetin changed the title [CLC-427]: Remove Data Migrations in Progress List and Status Map Usages [CLC-427][CLC-428]: Remove Data Migrations in Progress List and Status Map Usages Nov 6, 2023
# Conflicts:
#	base/commands/migration/const.go
#	base/commands/migration/start_stages.go
#	base/commands/migration/utils.go
@kutluhanmetin kutluhanmetin merged commit beb23e4 into hazelcast:dmt Nov 7, 2023
5 checks passed
@degerhz degerhz added this to the Data Migration Tool [Phase 1] milestone Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants