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

fix: race condition could cause mkdirs() to fail with "file exists" #869

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

0x29a
Copy link
Contributor

@0x29a 0x29a commented Jul 14, 2023

Exactly the same issue as #649, but with Studio:

*** Operational MODE: preforking ***
spawned uWSGI worker 1 (pid: 7, cores: 1)
spawned uWSGI worker 2 (pid: 9, cores: 1)
Traceback (most recent call last):
  File "cms/wsgi.py", line 21, in <module>
    import cms.startup as startup  # lint-amnesty, pylint: disable=wrong-import-position
  File "/openedx/edx-platform/./cms/startup.py", line 10, in <module>
    settings.INSTALLED_APPS  # pylint: disable=pointless-statement
  File "/openedx/venv/lib/python3.8/site-packages/django/conf/__init__.py", line 82, in __getattr__
    self._setup(name)
  File "/openedx/venv/lib/python3.8/site-packages/django/conf/__init__.py", line 69, in _setup
    self._wrapped = Settings(settings_module)
  File "/openedx/venv/lib/python3.8/site-packages/django/conf/__init__.py", line 170, in __init__
    mod = importlib.import_module(self.SETTINGS_MODULE)
  File "/opt/pyenv/versions/3.8.12/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "/openedx/edx-platform/./cms/envs/tutor/production.py", line 260, in <module>
    os.makedirs(folder)
  File "/opt/pyenv/versions/3.8.12/lib/python3.8/os.py", line 223, in makedirs
    mkdir(name, mode)
FileExistsError: [Errno 17] File exists: '/openedx/data/logs'
unable to load app 0 (mountpoint='') (callable not found or import error)
*** no app loaded. going in full dynamic mode ***

Also, this doesn't happen when OPENEDX_CMS_UWSGI_WORKERS is set to 1.

0x29a added a commit to open-craft/tutor that referenced this pull request Jul 14, 2023
@kaustavb12
Copy link
Contributor

@0x29a Can you please add a changelog entry too.

Copy link
Contributor

@kaustavb12 kaustavb12 left a comment

Choose a reason for hiding this comment

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

👍

@0x29a I tested this change in a sandbox and it solved the issue. This is approved from my end subject to the resolution of my comment above.

  • I tested this: Deployed and tested in sandbox
  • I read through the code

@0x29a
Copy link
Contributor Author

0x29a commented Jul 18, 2023

Thanks for the review, @kaustavb12. I added the entry.

cc @regisb

0x29a added a commit to open-craft/tutor that referenced this pull request Jul 19, 2023
See this PR for more information: overhangio#869

(cherry picked from commit eeb7b17)
@@ -18,6 +18,7 @@ Every user-facing change should have an entry in this changelog. Please respect

## Unreleased
- [Improvement] Make it possible to override k8s resources in plugins using `k8s-override` patch. (by @foadlind)
- [Bugfix] Fix a race condition that could prevent a newly provisioned Studio container from starting due to a FileExistsError when creating logs directory.
Copy link
Contributor

Choose a reason for hiding this comment

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

As indicated at the top of the changelog file, please don't add entries here. Instead, create new changelog entries with make changelog-entry. See: https://docs.tutor.overhang.io/tutor.html#contributing

Copy link
Contributor

Choose a reason for hiding this comment

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

I'll merge as-is and fix the changelog manually.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, @regisb and sorry, I didn't get to fixing this. I'll keep this in mind.

@regisb regisb merged commit df07422 into overhangio:master Aug 16, 2023
1 check passed
@0x29a 0x29a deleted the fix-dir-race-condition branch August 17, 2023 09:04
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.

3 participants