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

[#12048] Prepare for migration testing #13089

Conversation

FergusMok
Copy link
Contributor

@FergusMok FergusMok commented Apr 24, 2024

Part of #12048

  1. Add IndexCourseFields and relevant indexes.
  2. Added DataMigrationForCourseEntitySql50Newest and DataMigrationForCourseEntitySql50Oldest

Copy link
Contributor Author

@FergusMok FergusMok left a comment

Choose a reason for hiding this comment

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

Same for Oldest script

private static final int BATCH_SIZE = 100;

private static final int MAX_BUFFER_SIZE = 1000;
private static final AtomicLong NUMBER_TO_MIGRATE = new AtomicLong(50);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Important parts to note

Comment on lines +119 to +122
protected Query<Course> getFilterQuery() {
return ofy().load().type(teammates.storage.entity.Course.class)
.filter("isMigrated", false)
.order("-createdAt");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Descending time of created

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same for Oldest, where it's ascending

Comment on lines +690 to +692
if (numberOfScannedKey == NUMBER_TO_MIGRATE) {
shouldContinue = false;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stops once we hit the number to migrate

Copy link
Contributor

@mingyuanc mingyuanc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR

@FergusMok FergusMok merged commit 01b3209 into TEAMMATES:v9-course-migration Apr 24, 2024
1 check passed
@NicolasCwy NicolasCwy added c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging labels Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToMerge The PR is approved by all reviewers including final reviewer; ready for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants