Skip to content
This repository was archived by the owner on May 6, 2024. It is now read-only.

Unindent a few entries that needd to be unindented (#6450)#6467

Merged
pomegranited merged 1 commit intoopenedx-unsupported:open-release/lilac.masterfrom
BbrSofiane:bbrsofiane/fix-ansible-issue
Jul 6, 2021
Merged

Unindent a few entries that needd to be unindented (#6450)#6467
pomegranited merged 1 commit intoopenedx-unsupported:open-release/lilac.masterfrom
BbrSofiane:bbrsofiane/fix-ansible-issue

Conversation

@BbrSofiane
Copy link

(cherry picked from commit 945fe9d)

Configuration Pull Request

Make sure that the following steps are done before merging:

  • A DevOps team member has approved the PR if it is code shared across multiple services and you don't own all of the services.
  • Are you adding any new default values that need to be overridden when this change goes live? If so:
    • Update the appropriate internal repo (be sure to update for all our environments)
    • If you are updating a secure value rather than an internal one, file a DEVOPS ticket with details.
    • Add an entry to the CHANGELOG.
  • If you are making a complicated change, have you performed the proper testing specified on the Ops Ansible Testing Checklist? Adding a new variable does not require the full list (although testing on a sandbox is a great idea to ensure it links with your downstream code changes).
  • Think about how this change will affect Open edX operators. Have you updated the wiki page for the next Open edX release?

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Jul 4, 2021
@openedx-webhooks
Copy link

Thanks for the pull request, @BbrSofiane! I've created OSPR-5901 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@BbrSofiane
Copy link
Author

@regisb I've cherry-picked the commit.

Copy link
Contributor

@regisb regisb left a comment

Choose a reason for hiding this comment

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

note: I suggested this change but I have no idea how to check this is correct.

@regisb
Copy link
Contributor

regisb commented Jul 4, 2021

@edx-community-bot merge

(please)

@regisb
Copy link
Contributor

regisb commented Jul 4, 2021

This does not look like it's working... cc @sarina

@regisb
Copy link
Contributor

regisb commented Jul 5, 2021

cc @nedbat

@pomegranited
Copy link
Contributor

Thanks for submitting this fix @BbrSofiane @regisb -- I'm going to test it with our periodic build system, first with the current lilac.master (to watch it fail), and then with this change (to make sure it fixes it). Will get it merged ASAP!

Copy link
Contributor

@pomegranited pomegranited left a comment

Choose a reason for hiding this comment

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

👍 Thanks @BbrSofiane !

  • I tested this
    • deployed a sandbox on Ocim with open-release/lilac.master, and watched it fail at
      TASK [edxapp : install single-beat to run only one celerybeat scheduler]       
      ****************************************************************************************************
      fatal: [149.202.181.235]: FAILED! => {
          "changed": false,
          "msg": "Unsupported parameters for (pip) module: become_user, tags, when Supported parameters include: chdir, editable, executable, extra_args, name, requirements, state, umask, use_mirrors, version, virtualenv, virtualenv_command, virtualenv_python, virtualenv_site_packages"
      }
      
    • updated the sandbox to use this PR branch for deployment, and deployment passed that step without error.
  • I checked the code against the original https://github.com/edx/configuration/pull/6450

@pomegranited pomegranited merged commit 8e2c77b into openedx-unsupported:open-release/lilac.master Jul 6, 2021
@openedx-webhooks
Copy link

@BbrSofiane 🎉 Your pull request was merged!

Please take a moment to answer a two question survey so we can improve your experience in the future.

gabor-boros pushed a commit to open-craft/configuration that referenced this pull request Jul 7, 2021
…ed#6450) (openedx-unsupported#6467)

(cherry picked from commit 945fe9d)

Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
gabor-boros added a commit to open-craft/configuration that referenced this pull request Jul 12, 2021
The current implementation of celerybeat usage works properly if and
only if one concurrent worker process runs per queue using the nested
beat process started by the `--beat` flag. In case the the celerybeat
process is running as a separate process not as part of the workers,
that partially solves the problem. Either in that case, if multiple
instances are running in a cluster, without proper process supervising
across instances will result in (at least) dumplicated scheduling,
meaning that the scheduler will cause more work for the workers and
results will be duplicated.

Taking the above into consideration, to solve this issue, this commit
introduces single-beat package that wraps the celerybeat process and
keeps track of the process that acquired the lock in redis. In case one
of the instances get killed or the locking process crashes, the lock

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

Unindent a few entries that needd to be unindented (openedx-unsupported#6450) (openedx-unsupported#6467)
(cherry picked from commit 945fe9d)
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
gabor-boros added a commit to open-craft/configuration that referenced this pull request Jul 12, 2021
The current implementation of celerybeat usage works properly if and
only if one concurrent worker process runs per queue using the nested
beat process started by the `--beat` flag. In case the the celerybeat
process is running as a separate process not as part of the workers,
that partially solves the problem. Either in that case, if multiple
instances are running in a cluster, without proper process supervising
across instances will result in (at least) dumplicated scheduling,
meaning that the scheduler will cause more work for the workers and
results will be duplicated.

Taking the above into consideration, to solve this issue, this commit
introduces single-beat package that wraps the celerybeat process and
keeps track of the process that acquired the lock in redis. In case one
of the instances get killed or the locking process crashes, the lock

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

Unindent a few entries that needd to be unindented (openedx-unsupported#6450) (openedx-unsupported#6467)
(cherry picked from commit 945fe9d)
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
@natabene
Copy link
Contributor

Thank you all!

jdmulloy pushed a commit that referenced this pull request Jul 15, 2021
The current implementation of celerybeat usage works properly if and
only if one concurrent worker process runs per queue using the nested
beat process started by the `--beat` flag. In case the the celerybeat
process is running as a separate process not as part of the workers,
that partially solves the problem. Either in that case, if multiple
instances are running in a cluster, without proper process supervising
across instances will result in (at least) dumplicated scheduling,
meaning that the scheduler will cause more work for the workers and
results will be duplicated.

Taking the above into consideration, to solve this issue, this commit
introduces single-beat package that wraps the celerybeat process and
keeps track of the process that acquired the lock in redis. In case one
of the instances get killed or the locking process crashes, the lock

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

Unindent a few entries that needd to be unindented (#6450) (#6467)
(cherry picked from commit 945fe9d)
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
gabor-boros added a commit to edx-olive/configuration-old that referenced this pull request Oct 11, 2021
The current implementation of celerybeat usage works properly if and
only if one concurrent worker process runs per queue using the nested
beat process started by the `--beat` flag. In case the the celerybeat
process is running as a separate process not as part of the workers,
that partially solves the problem. Either in that case, if multiple
instances are running in a cluster, without proper process supervising
across instances will result in (at least) dumplicated scheduling,
meaning that the scheduler will cause more work for the workers and
results will be duplicated.

Taking the above into consideration, to solve this issue, this commit
introduces single-beat package that wraps the celerybeat process and
keeps track of the process that acquired the lock in redis. In case one
of the instances get killed or the locking process crashes, the lock

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

Unindent a few entries that needd to be unindented (openedx-unsupported#6450) (openedx-unsupported#6467)
(cherry picked from commit 945fe9d)
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
johanseto pushed a commit to eduNEXT/configuration that referenced this pull request Mar 18, 2022
The current implementation of celerybeat usage works properly if and
only if one concurrent worker process runs per queue using the nested
beat process started by the `--beat` flag. In case the the celerybeat
process is running as a separate process not as part of the workers,
that partially solves the problem. Either in that case, if multiple
instances are running in a cluster, without proper process supervising
across instances will result in (at least) dumplicated scheduling,
meaning that the scheduler will cause more work for the workers and
results will be duplicated.

Taking the above into consideration, to solve this issue, this commit
introduces single-beat package that wraps the celerybeat process and
keeps track of the process that acquired the lock in redis. In case one
of the instances get killed or the locking process crashes, the lock

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

refactor: use boolean flag to enable or disable celery beat
Co-authored-by: Joseph Mulloy <jdmulloy@users.noreply.github.com>

Unindent a few entries that needd to be unindented (openedx-unsupported#6450) (openedx-unsupported#6467)
(cherry picked from commit 945fe9d)
Co-authored-by: Chris Pappas <christopappas@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

merged open-source-contribution PR author is not from Axim or 2U

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants