Skip to content

Conversation

@iamsobanjaved
Copy link
Contributor

@iamsobanjaved iamsobanjaved commented Nov 10, 2020

This is the new PR for celery routing as that revert PR's branch name had too many reverts in it. We are currently facing an UnregisteredTask exception on the sandbox after rebasing with the latest master. Although this exception disappears after restarting sandbox by sudo /edx/bin/supervisorctl restart all multiple times

Now that fix is merged we can merge this.

This reverts commit c1fe3c3.
(cherry picked from commit db4c3b1)
https://github.com/edx/edx-platform/pull/25565

@iamsobanjaved iamsobanjaved force-pushed the iamsobanajved/celery-routing-update branch from 8d6bf21 to 820f2a9 Compare November 11, 2020 06:40
@iamsobanjaved iamsobanjaved changed the title WIP Celery routing upgrade Update celery routing for celery 4+ Nov 11, 2020
@awais786 awais786 self-requested a review November 11, 2020 13:25
@awais786
Copy link
Contributor

awais786 commented Nov 11, 2020

After rebasing PR some thing went wrong. Fresh sandbox throwing error.
https://awais786.sandbox.edx.org/

The full contents of the message body was:
b'{"task": "lms.djangoapps.instructor_task.tasks.calculate_grades_csv", "id": "b35980b0-9724-49c7-947e-d28761b8bcc8", "args": [1, {"xqueue_callback_url_prefix": "https://iamsobanjaved.sandbox.edx.org", "request_info": {"username": "staff", "user_id": 13, "ip": "111.119.187.19", "agent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.183 Safari/537.36", "host": "iamsobanjaved.sandbox.edx.org"}, "task_id": "b35980b0-9724-49c7-947e-d28761b8bcc8"}], "kwargs": {}, "group": null, "group_index": null, "retries": 0, "eta": null, "expires": null, "utc": true, "callbacks": null, "errbacks": null, "timelimit": [null, null], "taskset": null, "chord": null}' (708b)
Traceback (most recent call last):
  File "/edx/app/edxapp/venvs/edxapp/lib/python3.8/site-packages/celery/worker/consumer/consumer.py", line 562, in on_task_received
    strategy = strategies[type_]
KeyError: 'lms.djangoapps.instructor_task.tasks.calculate_grades_csv'

Tasks list not showing instructor tasks.


http://iamsobanjaved.sandbox.edx.org/

Another sandbox had the same issue but after multiple restarts and adding celery packages with our custom loggings soban sandbox now showing instructor tasks.

but interesting thing it is showing these instructor tasks (16 in total ) multiple times, appearing in high queue, highmem queue and default queue also.

@nedbat
Copy link
Contributor

nedbat commented Nov 12, 2020

@jmbowman This is the latest PR for the routing changes. Is it bad to make Koa without it?

@iamsobanjaved
Copy link
Contributor Author

iamsobanjaved commented Nov 12, 2020

We have made sandbox with master and it was working fine, but after some restarts, it has started to showing UnregisteredTask exception on Instructor tasks. Even though it is showing registered when we run this DJANGO_SETTINGS_MODULE=lms.envs.production /edx/app/edxapp/venvs/edxapp/bin/python3.8 /edx/app/edxapp/venvs/edxapp/bin/celery --app=lms.celery:APP inspect registered

I think the instructor task config is flaky here, as we are not importing tasks in AppConfig of this app.

The output from this command:
-> celery@edx.lms.core.high_mem.mastertest: OK * celery_utils.tasks.mark_resolved * edx_sga.tasks.zip_student_submissions * enterprise.tasks.create_enterprise_enrollment * entitlements.expire_old_entitlements [routing_key=edx.lms.core.default] * integrated_channels.integrated_channel.tasks.transmit_content_metadata * integrated_channels.integrated_channel.tasks.transmit_learner_data * integrated_channels.integrated_channel.tasks.transmit_single_learner_data * integrated_channels.integrated_channel.tasks.unlink_inactive_learners * lms.djangoapps.bulk_email.tasks.send_course_email * lms.djangoapps.ccx.tasks.send_ccx_course_published * lms.djangoapps.certificates.tasks.generate_certificate * lms.djangoapps.discussion.tasks.send_ace_message [routing_key=edx.lms.core.default] * lms.djangoapps.discussion.tasks.update_discussions_map * lms.djangoapps.email_marketing.tasks.get_email_cookies_via_sailthru [routing_key=edx.lms.core.default] * lms.djangoapps.email_marketing.tasks.update_course_enrollment [routing_key=edx.lms.core.default] * lms.djangoapps.email_marketing.tasks.update_user [routing_key=edx.lms.core.default] * lms.djangoapps.email_marketing.tasks.update_user_email [routing_key=edx.lms.core.default] * lms.djangoapps.gating.tasks.task_evaluate_subsection_completion_milestones * lms.djangoapps.grades.tasks.compute_all_grades_for_course [routing_key=edx.lms.core.default] * lms.djangoapps.grades.tasks.compute_grades_for_course * lms.djangoapps.grades.tasks.compute_grades_for_course_v2 [rate_limit=300/h] * lms.djangoapps.grades.tasks.recalculate_course_and_subsection_grades_for_user [routing_key=edx.lms.core.default] * lms.djangoapps.grades.tasks.recalculate_subsection_grade_v3 [routing_key=edx.lms.core.default] * lms.djangoapps.instructor_task.tasks.calculate_grades_csv [routing_key=edx.lms.core.high_mem] * lms.djangoapps.instructor_task.tasks.calculate_may_enroll_csv * lms.djangoapps.instructor_task.tasks.calculate_problem_grade_report [routing_key=edx.lms.core.high_mem] * lms.djangoapps.instructor_task.tasks.calculate_problem_responses_csv.v2 [routing_key=edx.lms.core.high_mem] * lms.djangoapps.instructor_task.tasks.calculate_students_features_csv * lms.djangoapps.instructor_task.tasks.cohort_students * lms.djangoapps.instructor_task.tasks.course_survey_report_csv * lms.djangoapps.instructor_task.tasks.delete_problem_state * lms.djangoapps.instructor_task.tasks.export_ora2_data * lms.djangoapps.instructor_task.tasks.export_ora2_submission_files * lms.djangoapps.instructor_task.tasks.generate_certificates [routing_key=edx.lms.core.high_mem] * lms.djangoapps.instructor_task.tasks.override_problem_score * lms.djangoapps.instructor_task.tasks.proctored_exam_results_csv * lms.djangoapps.instructor_task.tasks.rescore_problem * lms.djangoapps.instructor_task.tasks.reset_problem_attempts * lms.djangoapps.instructor_task.tasks.send_bulk_course_email * lms.djangoapps.program_enrollments.tasks.expire_waiting_enrollments * lms.djangoapps.verify_student.tasks.send_request_to_ss_for_user [routing_key=edx.lms.core.high] * lms.djangoapps.verify_student.tasks.send_verification_status_email [routing_key=edx.lms.core.default] * openedx.core.djangoapps.content.block_structure.tasks.get_course_in_cache * openedx.core.djangoapps.content.block_structure.tasks.get_course_in_cache_v2 * openedx.core.djangoapps.content.block_structure.tasks.update_course_in_cache * openedx.core.djangoapps.content.block_structure.tasks.update_course_in_cache_v2 * openedx.core.djangoapps.coursegraph.tasks.dump_course_to_neo4j [routing_key=edx.lms.core.default] * openedx.core.djangoapps.credentials.tasks.v1.tasks.send_grade_to_credentials [routing_key=edx.lms.core.default] * openedx.core.djangoapps.programs.tasks.award_course_certificate [routing_key=edx.lms.core.default] * openedx.core.djangoapps.programs.tasks.award_program_certificates [routing_key=edx.lms.core.default] * openedx.core.djangoapps.programs.tasks.revoke_program_certificates [routing_key=edx.lms.core.default] * openedx.core.djangoapps.programs.tasks.update_certificate_visible_date_on_course_update [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks.ScheduleCourseNextSectionUpdate [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks.ScheduleCourseUpdate [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks.ScheduleRecurringNudge [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks.ScheduleUpgradeReminder [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks._course_update_schedule_send [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks._recurring_nudge_schedule_send [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks._upgrade_reminder_schedule_send [routing_key=edx.lms.core.default] * openedx.core.djangoapps.schedules.tasks.update_course_schedules * openedx.core.djangoapps.verified_track_content.tasks.sync_cohort_with_mode * openedx.features.enterprise_support.tasks.clear_enterprise_customer_data_consent_share_cache * student.send_activation_email * super_csv.mixins.do_deferred_commit * super_csv.tasks.expire_data * third_party_auth.fetch_saml_metadata

Sandbox: https://mastertest.sandbox.edx.org/
Flower: http://54.242.239.66:5555/dashboard

@awais786
Copy link
Contributor

For a sandbox you can try these commands it will execute the instructor tasks 'generate-grade-csv' and show the result.

@jmbowman
Copy link
Contributor

@nedbat I'm pretty sure we want this in Koa, but at this point we should just cherrypick the commit once we get the whole problem sorted out. It shouldn't block any testing unless somebody wants to verify that tasks are still going to the correct queues (since without this they all go to the default queue).

@iamsobanjaved Yeah, the terrible part of this is that the problem is intermittent even within the same deployment, and celery gives no useful information whatsoever to help in debugging it. I think we need to get some results from BOM-2086 before we can safely merge either this or the virtualenv upgrade again.

@awais786 awais786 force-pushed the iamsobanajved/celery-routing-update branch 4 times, most recently from a323a33 to 2d66aff Compare November 18, 2020 12:44
@iamsobanjaved iamsobanjaved force-pushed the iamsobanajved/celery-routing-update branch from 2d66aff to 0674753 Compare November 23, 2020 09:12
timmc-edx added a commit that referenced this pull request Dec 8, 2020
…ate' into timmc/celery-debug

PR #25567 at 2020-12-08

# Conflicts:
#	cms/envs/production.py
#	lms/envs/production.py
@jmbowman
Copy link
Contributor

Now that https://github.com/edx/edx-platform/pull/25840 has been merged, we should be clear to rebase and merge this. That PR made some changes to these files, so you may need to refactor a bit.

awais786 and others added 2 commits December 14, 2020 11:35
- Used routing function istead of class
- Move task queues to Djano settings
- Removed routing_key parameter
@iamsobanjaved iamsobanjaved force-pushed the iamsobanajved/celery-routing-update branch from 0674753 to f28b218 Compare December 14, 2020 06:53
@edx-status-bot
Copy link

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

@awais786 awais786 marked this pull request as ready for review December 15, 2020 06:45
@iamsobanjaved
Copy link
Contributor Author

After the fix, rebased it with the latest master and refactored it. Tested it on multiple sandboxes and routing is working as per expectations.

Sandbox: https://celeryroute2.sandbox.edx.org/
Flower: http://52.91.58.200:5555/dashboard

Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

This looks great. It looks as a result of this change, how people write celery tasks for LMS and CMS will change. I think a follow-on task to write a new platform level docs/howto(How to add new celery tasks) would be a valuable doc to add.

Please make sure that that work is ticketed and done soon.

@iamsobanjaved iamsobanjaved merged commit bd601cf into master Dec 16, 2020
@iamsobanjaved iamsobanjaved deleted the iamsobanajved/celery-routing-update branch December 16, 2020 08:40
@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.

@jmbowman
Copy link
Contributor

@nedbat Do we want to cherry pick this into Koa to fix celery task routing?

gabor-boros pushed a commit to open-craft/openedx-platform that referenced this pull request May 15, 2021
* Update celery routing

- Used routing function instead of class
- Move task queues dictionary to Django settings
- Removed routing_key parameter
- Refactored routing for singleton celery instantiation

Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
gabor-boros pushed a commit to open-craft/openedx-platform that referenced this pull request May 17, 2021
* Update celery routing

- Used routing function instead of class
- Move task queues dictionary to Django settings
- Removed routing_key parameter
- Refactored routing for singleton celery instantiation

Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
nedbat pushed a commit that referenced this pull request May 18, 2021
* Update celery routing

- Used routing function instead of class
- Move task queues dictionary to Django settings
- Removed routing_key parameter
- Refactored routing for singleton celery instantiation

Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
pomegranited pushed a commit to open-craft/openedx-platform that referenced this pull request Jun 24, 2021
* Update celery routing

- Used routing function instead of class
- Move task queues dictionary to Django settings
- Removed routing_key parameter
- Refactored routing for singleton celery instantiation

Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
(cherry picked from commit e3b4d23)
gabor-boros added a commit to open-craft/openedx-platform that referenced this pull request Jul 14, 2021
* [SE-4101] fix: address VisibleBlocks caching race condition (openedx#27359) (#349)

* fix: address VisibleBlocks caching race condition
* sets visual block creation in an atomic transaction
* refactor: add logging statement to bulk create

Co-authored-by: Raul Gallegos <raul@opencraft.com>

Co-authored-by: Raul Gallegos <raul@opencraft.com>

* BB-3954 Add toggle for enrollment behavior (#351)

Adds toggle REDIRECT_UNAUTHENTICATED_USER_TO_LOGIN_ON_ENROLL
for enrollment behaviour for unauthenticated user.
If true, the user will be redirected to 'signin_user' route.

Co-authored-by: Arjun Singh Yadav <arjun@opencraft.com>

* [BB-3622] feat:Restrict user create course (#319) (#352)

* Add course creation condition for organization

The condition added to ensure that if the feature is enabled
user will not be able to create the course outside of the organization
in which they belong.

Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com>

* fix: update the xblock-lti-consumer commit

* Added new setting CERTIFICATE_DATE_FORMAT for easy customization of (#354)

certificate issued date

(cherry picked from commit 421e661)

* fix: Produce grade report when subsections have future start dates

When getting a subsection grade for a user, instead of failing
if the user can't access that subsection,
fallback to the collected structure.

* Revert "Unhide student-generated certificates toggle"

This reverts commit 8910ccf.

Our clients are no longer using this feature, and edX won't accept this
upstream as-is currently.  See https://github.com/edx/edx-platform/pull/23735 for more information.

Reverting to reduce code drift from upstream.

* chore: update Arabic translations

* fix:Fix function call to check MFE (#359)

Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com>

* fix: don't cache ajax request

* feat: add site language config

Cherry-picked from: https://github.com/edx/edx-platform/pull/27696

* Revert "[FAL-1813] fix: codejail issue when using matplotlib (#343)" (#365)

This reverts commit 2143f84.

* Update celery routing for celery 4+ (openedx#25567)

* Update celery routing

- Used routing function instead of class
- Move task queues dictionary to Django settings
- Removed routing_key parameter
- Refactored routing for singleton celery instantiation

Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
(cherry picked from commit e3b4d23)

* fix: add missing set_code_owner_attribute imports

(cherry picked from commit d1060d2)

* chore: bump edx-django-utils to 3.12.0

The required `set_code_owner_attribute` decorator was introduced in
v3.12.0, therefore we need to bump the dependency.

(cherry picked from commit f52b84e)

* [SE-4482] Allow delete course content in Studio only for admin users (#360)

Co-authored-by: Nizar Mahmoud <nizarmah@hotmail.com>

* fix: Password reset page throwing not found error (#364)

Signed-off-by: Farhaan Bukhsh <farhaan@opencraft.com>

* fix: change buttons in wiki modal to anchor tags

The action buttons in wiki modal have href attribute
but does not have an event listener for click.
This PR changes the buttons to anchor tags so that they
work as expected when clicked.

* fix: use high priority queue for celery heartbeat check

Makes HIGH_PRIORITY_QUEUE a derived setting, which allows
HEARTBEAT_CELERY_ROUTING_KEY to use the correct config variant default.

(cherry picked from commit 2a9067a)

* fix: prevent invalidation of allowlisted certificates

The allowlisted certificates were getting invalidated upon visiting the Course
Progress page by users.

This is a rough backport of the Lilac fix (edx#26356). In Lilac, this is gated
by the `certificates_revamp.use_allowlist` Waffle flag. In post-Lilac branches,
this is working out of the box (the flag has been removed in edx#27576).

Jira ticket: BB-4287

* feat: add celery beat configuration

Co-authored-by: Raul Gallegos <raul@opencraft.com>
Co-authored-by: Farhaan Bukhsh <farhaan@opencraft.com>
Co-authored-by: Arjun Singh Yadav <arjun@opencraft.com>
Co-authored-by: pkulkark <pooja@opencraft.com>
Co-authored-by: Shimul Chowdhury <shimul@opencraft.com>
Co-authored-by: João Cabrita <joao.cabrita@opencraft.com>
Co-authored-by: Samuel Walladge <samuel@opencraft.com>
Co-authored-by: Giovanni Cimolin da Silva <giovannicimolin@gmail.com>
Co-authored-by: 0x29a <demid@opencraft.com>
Co-authored-by: Dmitry Gamanenko <dmitry.gamanenko@raccoongang.com>
Co-authored-by: Jillian Vogel <jill@opencraft.com>
Co-authored-by: Muhammad Soban Javed <58461728+iamsobanjaved@users.noreply.github.com>
Co-authored-by: Sandeep Choudhary <sandeep@opencraft.com>
Co-authored-by: Nizar Mahmoud <nizarmah@hotmail.com>
Co-authored-by: Agrendalath <piotr@surowiec.it>
sambapete added a commit to EDUlib/edx-platform that referenced this pull request Aug 9, 2021
* Update celery routing

- Used routing function instead of class
- Move task queues dictionary to Django settings
- Removed routing_key parameter
- Refactored routing for singleton celery instantiation

Co-authored-by: Awais Qureshi <awais.qureshi@arbisoft.com>
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.

8 participants