Skip to content

Revert: "Add a data migration to copy all course index data into MySQL"#29212

Merged
ormsbee merged 1 commit into
masterfrom
revert-29144-braden/course-indexes-mysql-4-data-migration
Nov 3, 2021
Merged

Revert: "Add a data migration to copy all course index data into MySQL"#29212
ormsbee merged 1 commit into
masterfrom
revert-29144-braden/course-indexes-mysql-4-data-migration

Conversation

@connorhaugh

Copy link
Copy Markdown
Contributor

@ormsbee ormsbee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@ashultz0

ashultz0 commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

this is the revert it says it is so 👍
checks seem to have run aground on dependency issues

@ormsbee

ormsbee commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

Error (with course key redacted):

      Applying split_modulestore_django.0002_data_migration...
  succeeded: false
  succeeded_migrations: []
  traceback: |
    Traceback (most recent call last):
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/release_util/management/commands/__init__.py”, line 280, in __apply
        call_command(“migrate”, **migrate_kwargs)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/management/__init__.py”, line 181, in call_command
        return command.execute(*args, **defaults)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/management/base.py”, line 398, in execute
        output = self.handle(*args, **options)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/management/base.py”, line 89, in wrapped
        res = handle_func(*args, **kwargs)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/management/commands/migrate.py”, line 244, in handle
        post_migrate_state = executor.migrate(
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/migrations/executor.py”, line 117, in migrate
        state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/migrations/executor.py”, line 147, in _migrate_all_forwards
        state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/migrations/executor.py”, line 227, in apply_migration
        state = migration.apply(state, schema_editor)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/migrations/migration.py”, line 123, in apply
        operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
      File “/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/migrations/operations/special.py”, line 190, in database_forwards
        self.code(from_state.apps, schema_editor)
      File “/edx/app/edxapp/edx-platform/common/djangoapps/split_modulestore_django/migrations/0002_data_migration.py”, line 34, in forwards_func
        raise ValueError(
    ValueError: Course course-v1:???+???+??? already exists in MySQL but the MongoDB version is newer. That’s
    unexpected because since the course index table was added to MySQL, there has never been a time when we
    would write course_indexes updates only to MongoDB without also writing to MySQL.

@ormsbee

ormsbee commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

Oh, could this be because it was rolled back at one point?

@edx-status-bot

Copy link
Copy Markdown

Your PR has finished running tests. There were no failures.

@bradenmacdonald

bradenmacdonald commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

@ormsbee I'm about to step out for a couple hours but can look at this later.

Oh, could this be because it was rolled back at one point?

Not sure, weren't the tables deleted when the original migration was rolled back? If the tables/data were kept, that could be it, yep.

Otherwise it'd be helpful to know how much newer the data in MySQL is than MongoDB. If it's only 1 second newer, it's likely a timing bug and not a big deal. But if it's significant, it means we may be missing some course writes and we'll need to investigate more.

I wasn't sure about including this condition+error in the migration as I knew there was a chance it would fail like this. But I thought it's better to be safe than sorry with data migrations, and to check our assumptions carefully.

@ormsbee
ormsbee merged commit b8d49e7 into master Nov 3, 2021
@ormsbee
ormsbee deleted the revert-29144-braden/course-indexes-mysql-4-data-migration branch November 3, 2021 17:53
@ormsbee

ormsbee commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

I wasn't sure about including this condition+error in the migration as I knew there was a chance it would fail like this. But I thought it's better to be safe than sorry with data migrations, and to check our assumptions carefully.

Maybe log loudly and we can investigate? Since it's still reading from MongoDB, it can't do that much harm, right?

@ormsbee

ormsbee commented Nov 3, 2021

Copy link
Copy Markdown
Contributor

NOTE: I merged this with admin rights because the two errors were unrelated (they look like possibly intermittent errors around fetching edx-jsme). In any event, the two failing test types (quality and JS) should be unaffected by a rollback of a database data migration.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

1 similar comment
@edx-pipeline-bot

Copy link
Copy Markdown
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

@bradenmacdonald

bradenmacdonald commented Nov 4, 2021

Copy link
Copy Markdown
Contributor

@ormsbee I'll prepare a new version of the PR that logs a detailed warning instead of throwing an exception, as you suggest. Sorry about the hassle here.

@ormsbee

ormsbee commented Nov 4, 2021

Copy link
Copy Markdown
Contributor

Okay, another weird thing is that I don't see the course ID in the SQL table at all in that environment (I double checked our other environments as well, just to see if I was somehow reading it wrong). The MongoDB entry for it is years old (ISODate("2016-07-18T08:48:59.430Z"))

@bradenmacdonald

Copy link
Copy Markdown
Contributor

@ormsbee Weird! I don't understand that. Is there any chance it could be listed in MongoDB twice somehow, so that it was temporarily created in the data migration transaction, and then a second time it was attempted to be created, this error was raised? The code is just mysql_entry = SplitModulestoreCourseIndex.objects.get(course_id=course_id) so I'm not sure how it could be finding something that's not there.

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.

6 participants