-
Notifications
You must be signed in to change notification settings - Fork 264
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
Concurrency issues(²?) #950
Comments
Not sure this is relevant, but: psycopg/psycopg2#840 (comment) |
Another workaround: |
And possibly related: #127, found from https://github.com/nautobot/nautobot/blob/v1.1.3/nautobot/docs/installation/services.md#ssl-error by searching |
@jathanism In #127 (comment), you mentioned that disabling SSL was a solution, I remembered reading at https://www.psycopg.org/docs/install.html#psycopg-vs-psycopg-binary that the source distribution should be used in production, otherwise some issues related to SSL may appear under concurrency -> have you tried to reproduce the issue by using After understanding the issue in #1515, I was interested to look at this again, but from my testing however, neither using I wanted to try the # nautobot-server
nautobot.core.cli.main()
# https://github.com/nautobot/nautobot/blob/v1.2.10/nautobot/core/cli.py
nautobot.core.runner.run_app()
# https://github.com/nautobot/nautobot/blob/v1.2.10/nautobot/core/runner/runner.py
django.core.management.execute_from_command_line()
# https://github.com/django/django/blob/stable/3.1.x/django/core/management/__init__.py
django_webserver.management.commands.pyuwsgi.Command.run_from_argv() # (from django_webserver.base_command.WebserverCommand)
# https://github.com/lincolnloop/django-webserver/blob/v1.2.0/django_webserver/management/commands/pyuwsgi.py
if WEBSERVER_WARMUP is True: # by default -> the WSGI_APPLICATION is called before uwsgi is loaded here
django.core.servers.basehttp.get_internal_wsgi_application()
django_webserver.management.commands.pyuwsgi.Command.prep_server_args()
django_webserver.management.commands.pyuwsgi.Command.start_server()
# https://github.com/lincolnloop/django-webserver/blob/v1.2.0/django_webserver/base_command.py
pyuwsgi.run()
# -> Will try to run WSGI_APPLICATION (= nautobot.core.wsgi:application)
# Now we have the uwsgi module loaded
# https://github.com/nautobot/nautobot/blob/v1.2.10/nautobot/core/wsgi.py
nautobot.setup()
django.core.wsgi.get_wsgi_application
django.setup(set_prefix=False)
# https://github.com/django/django/blob/stable/3.1.x/django/core/wsgi.py
django.core.handlers.wsgi.WSGIHandler()
# https://github.com/django/django/blob/stable/3.1.x/django/core/handlers/wsgi.py
# Now uwsgi loaded the application, it'll create threads https://code.djangoproject.com/ticket/20562#comment:12 (found via https://stackoverflow.com/questions/8242837/django-multiprocessing-and-database-connections) seems to talk about this startup process (forking from management command)? Before understanding this, I tried to hack around from django_webserver.management.commands.pyuwsgi import Command as uWSGICommand
"""
try:
import uwsgidecorators
@uwsgidecorators.postfork
def fix_uwsgi():
import logging
logging.getLogger("nautobot").warning('>>>>>>>>>>> close')
import django.db
django.db.connections.close_all()
except ImportError:
pass
"""
class Command(uWSGICommand):
help = "Start Nautobot uWSGI server."
def run_from_argv(self, argv):
import logging
logging.getLogger("nautobot").warning('>>>>>>>>>>> close')
import django.db
django.db.connections.close_all()
super().run_from_argv(argv) This seems to works, but it's not the best place to put this. After understanding the startup process, I tried the following solutions (separately), all of them seems to work too:
[...]
# Use try/except because we might not be running uWSGI.
# Like `django_webserver.base_command.WebserverCommand`, when
# `settings.WEBSERVER_WARMUP` is not set to `False`, will first call
# `get_internal_wsgi_application` which does not have `uwsgi` module loaded
# already -> you must set `settings.WEBSERVER_WARMUP` to `False` for this
# code to be loaded
try:
import uwsgidecorators
@uwsgidecorators.postfork
def fix_uwsgi():
import uwsgi, logging
logging.getLogger("nautobot").info(f'Closing existing DB and cache connections on worker {uwsgi.worker_id()} after uWSGI forked ...')
import django.db, django.core.cache
django.db.connections.close_all()
django.core.cache.close_caches()
except ImportError:
pass
application = [...] This should be the "correct" way to do it? But closing must be done on each fork, also
from django_webserver.management.commands.pyuwsgi import Command as uWSGICommand
class Command(uWSGICommand):
help = "Start Nautobot uWSGI server."
def start_server(self, *argv):
import logging
logging.getLogger("nautobot").info('Closing existing DB and cache connections before uWSGI forks ...')
import django.db, django.core.cache
django.db.connections.close_all()
django.core.cache.close_caches()
super().start_server(*argv) This seems to be working from my tests, this doesn't require to disable
[...]
# Use try/except because we might not be running uWSGI.
# Like `django_webserver.base_command.WebserverCommand`, when
# `settings.WEBSERVER_WARMUP` is not set to `False`, will first call
# `get_internal_wsgi_application` which does not have `uwsgi` module loaded
# already -> you must set `settings.WEBSERVER_WARMUP` to `False` for this
# code to be loaded
try:
import uwsgi
except ImportError:
pass
else:
# uwsgi is initialized and will fork when it gets `application`
import logging
logging.getLogger("nautobot").info('Closing existing DB and cache connections before uWSGI forks ...')
import django.db, django.core.cache
django.db.connections.close_all()
django.core.cache.close_caches()
application = [...] This should allow to remove exiting connections just before the fork, but it requires to disable
To test, I used To get a shell with docker-compose run --rm --entrypoint nautobot-server nautobot start --ini uwsgi.ini --pyshell (and I have about 50 links of source code that where useful to trace the startup process, but I won't spam here :) ) |
Hi, were you able to look into this? Maybe there's a refactoring/change to come and make this obsolete (like remove uWSGI)? Or would you accept a PR for any of those solutions (maybe solution n°2, Best, |
Able to reproduce in 1.4.1 (and to fix using at least the last solution, closing connections just before handing over
|
@u1735067 This issue got lost in the shuffle due to it already being groomed. I'm taking a look at this and try to wrap my head around your experience. Thanks for your patience for submitting your detailed analysis as you're known to do! |
After testing the fix with the following:
I have confirmed that this absolutely fixes this problem and will allow Nautobot to support SSL database connections WITH persistent connections. Thank you @u1735067 for your patience and for your time in debugging and getting to the bottom of this! I'll provide a patch soon and make sure you get the credit. For reference, it's this:
import logging
from django.core import cache
from django.core.wsgi import get_wsgi_application
from django.db import connections
import nautobot
# This is the Django default left here for visibility on how the Nautobot pattern
# differs.
# os.environ.setdefault("DJANGO_SETTINGS_MODULE", "nautobot.core.settings")
# Instead of importing `DJANGO_SETTINGS_MODULE` we're using the custom loader
# pattern from `nautobot.core.runner` to read environment or config path for us.
nautobot.setup()
# Use try/except because we might not be running uWSGI.
# Like `django_webserver.base_command.WebserverCommand`, when
# `settings.WEBSERVER_WARMUP` is not set to `False`, will first call
# `get_internal_wsgi_application` which does not have `uwsgi` module loaded
# already -> you must set `settings.WEBSERVER_WARMUP` to `False` for this
# code to be loaded
try:
import uwsgidecorators
@uwsgidecorators.postfork
def fix_uwsgi():
import uwsgi
logging.getLogger("nautobot").info(f'Closing existing DB and cache connections on worker {uwsgi.worker_id()} after uWSGI forked ...')
connections.close_all()
cache.close_caches()
except ImportError:
pass
application = get_wsgi_application() |
Environment
Docker-compose extract:
Steps to Reproduce
docker-compose down -v --remove-orphans
docker-compose up -d
docker-compose logs -f
ansible-inventory --list -i my_inventory.yml -vv | grep -v ansible_password > inv.json
Expected Behavior
Nautobot answers to all requests without errors, so the Ansible plugin can construct the consolidated inventory
Observed Behavior
Nautobot answers with random errors, like in #744
Additional informations
docker-compose logs
ansible stack trace
Some errors extract:
get() returned more than one ContentType -- it returned more than 20!
generator raised StopIteration
'>' not supported between instances of 'UUID' and 'int'
Setting
NAUTOBOT_UWSGI_PROCESSES
to1
seems to mitigate the issue, but makes Nautobot incredibly slow, with some requests (like/api/dcim/interfaces/?limit=0
, even withNAUTOBOT_MAX_PAGE_SIZE: "5000"
instead of50000
) falling in timeout (which if using Traefik as reverse-proxy, can make it return 404 momentarily due to the healthcheck https://github.com/nautobot/nautobot/blob/v1.1.3/docker/Dockerfile#L20 failing and Traefik removing the destination).I haven't tried to play with
NAUTOBOT_CACHEOPS_ENABLED
this time, it was commented in all tests.The text was updated successfully, but these errors were encountered: