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

Revert "Store split modulestore's course indexes in Django/MySQL" #28979

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

ormsbee
Copy link
Contributor

@ormsbee ormsbee commented Oct 7, 2021

Reverts edx/edx-platform#27565

@bradenmacdonald: The original PR broke migrations on stage. Will add more details later, but starting the revert build now.

@ormsbee ormsbee requested a review from a team October 7, 2021 17:23
@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 7, 2021

    Running migrations:
      Applying split_modulestore_django.0001_initial... OK
      Applying split_modulestore_django.0002_data_migration...
  succeeded: false
  succeeded_migrations:
  - - split_modulestore_django
    - 0001_initial
  traceback: |
    Traceback (most recent call last):
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
        return self.cursor.execute(sql, params)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 71, in execute
        return self.cursor.execute(query, args)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/MySQLdb/cursors.py", line 206, in execute
        res = self._query(query)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/MySQLdb/cursors.py", line 319, in _query
        db.query(q)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/MySQLdb/connections.py", line 259, in query
        _mysql.connection.query(self, query)
    MySQLdb._exceptions.IntegrityError: (1062, "Duplicate entry 'course-v1:edX+TL101+2015' for key 'course_id'")

    The above exception was the direct cause of the following exception:

    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 148, 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 364, in execute
        output = self.handle(*args, **options)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/core/management/base.py", line 83, 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 232, 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 245, 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 121, 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 22, in forwards_func
        SplitModulestoreCourseIndex(**data).save(using=db_alias)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py", line 743, in save
        self.save_base(using=using, force_insert=force_insert,
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py", line 780, in save_base
        updated = self._save_table(
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py", line 873, in _save_table
        result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/base.py", line 910, in _do_insert
        return manager._insert([self], fields=fields, return_id=update_pk,
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/manager.py", line 82, in manager_method
        return getattr(self.get_queryset(), name)(*args, **kwargs)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/query.py", line 1186, in _insert
        return query.get_compiler(using=using).execute_sql(return_id)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/models/sql/compiler.py", line 1377, in execute_sql
        cursor.execute(sql, params)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py", line 67, in execute
        return self._execute_with_wrappers(sql, params, many=False, executor=self._execute)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py", line 76, in _execute_with_wrappers
        return executor(sql, params, many, context)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
        return self.cursor.execute(sql, params)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/utils.py", line 89, in __exit__
        raise dj_exc_value.with_traceback(traceback) from exc_value
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/utils.py", line 84, in _execute
        return self.cursor.execute(sql, params)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/django/db/backends/mysql/base.py", line 71, in execute
        return self.cursor.execute(query, args)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/MySQLdb/cursors.py", line 206, in execute
        res = self._query(query)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/MySQLdb/cursors.py", line 319, in _query
        db.query(q)
      File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/MySQLdb/connections.py", line 259, in query
        _mysql.connection.query(self, query)
    django.db.utils.IntegrityError: (1062, "Duplicate entry 'course-v1:edX+TL101+2015' for key 'course_id'")

Copy link
Member

@kdmccormick kdmccormick left a comment

Choose a reason for hiding this comment

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

Bummer!

The split_modulestore_django.0001_initial migration successfully applied on Stage. I assume that reverting that migration (and then later reintroducing it) won't cause any issues?

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 7, 2021

@kdmccormick: Yeah, I think that'll be fine.

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 7, 2021

@bradenmacdonald: I suspect this may be because MongoDB is case sensitive on course_id and MySQL is not. (Trying to verify if this is the issue with this data now.)

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 7, 2021

Yeah, it’s totally the case sensitivity thing (capitalization difference in the orgs):

stage-edx> db.modulestore.active_versions.find({"course": "TL101"}, {org: 1, course: 1, run: 1})
{ "_id" : ObjectId("553115a9d15a010b5c6f7228"), "run" : "2015", "org" : "edx", "course" : "TL101" }
{ "_id" : ObjectId("550869e42d00970b5b082d2a"), "run" : "2015", "org" : "edX", "course" : "TL101" }

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 7, 2021

@bradenmacdonald: Just as a heads up, edX will be running with limited staffing next week, so my new target deploy date for this would be Oct. 18.

@bradenmacdonald
Copy link
Contributor

@ormsbee Argh, ok. Well I knew the probability of something like that was high. Thanks for taking care of the deploy and revert today.

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 7, 2021

I'm merging this despite quality failures, cause, revert.

@ormsbee ormsbee merged commit ae124bd into master Oct 7, 2021
@ormsbee ormsbee deleted the revert-27565-braden/course-indexes-mysql branch October 7, 2021 19:07
@edx-pipeline-bot
Copy link
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
Contributor

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

@edx-pipeline-bot
Copy link
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
Contributor

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

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

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

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 13, 2021

@bradenmacdonald: Another thought for your consideration–I believe the current PR as it stands would introduce a period of time in which writes can get lost (i.e. the gap between when the migrations have run and all the frontends have switched over to the new code). It's only a gap of ten minutes or so, but depending on time of day, that could still result in hundreds of lost edits. The two mitigations I can think of for this are to temporarily bring down Studio or to re-order the elements of this PR so that the code prefers MongoDB for some time until we're sure all the data is migrated over.

Thoughts?

@bradenmacdonald
Copy link
Contributor

@ormsbee Good point, I wasn't sure how long that time period would be and assumed it was quite short. I think if we change this to read from MongoDB but to write to both (MySQL + MongoDB), we could deploy it without the data migration, and then later run the data migration only for courses which haven't been written to since the original deployment. I think that would avoid any gaps. What do you think?

@ormsbee
Copy link
Contributor Author

ormsbee commented Oct 15, 2021

That sounds great.

@bradenmacdonald
Copy link
Contributor

@ormsbee OK, I'll do that. Unfortunately I didn't get time to do it for Monday AM but I'll try to get it ready for Tuesday.

@bradenmacdonald
Copy link
Contributor

Yeah, it’s totally the case sensitivity thing (capitalization difference in the orgs):

stage-edx> db.modulestore.active_versions.find({"course": "TL101"}, {org: 1, course: 1, run: 1})
{ "_id" : ObjectId("553115a9d15a010b5c6f7228"), "run" : "2015", "org" : "edx", "course" : "TL101" }
{ "_id" : ObjectId("550869e42d00970b5b082d2a"), "run" : "2015", "org" : "edX", "course" : "TL101" }

@ormsbee Just to be clear, are those two different courses or two different records about the same course? Is this just a concern for migration or for the general case of using this?

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