From fa77be6e6c2b8da564121e2ca698a29ae74562f3 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Fri, 25 Mar 2022 18:29:18 -0700 Subject: [PATCH] upgrade: Only run Django system checks once, explicitly. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are expensive, and moving them to one explicit call early has considerable time savings in the critical period: ``` $ hyperfine './manage.py fill_memcached_caches' './manage.py fill_memcached_caches --skip-checks' Benchmark #1: ./manage.py fill_memcached_caches Time (mean ± σ): 5.264 s ± 0.146 s [User: 4.885 s, System: 0.344 s] Range (min … max): 5.119 s … 5.569 s 10 runs Benchmark #2: ./manage.py fill_memcached_caches --skip-checks Time (mean ± σ): 3.090 s ± 0.089 s [User: 2.853 s, System: 0.214 s] Range (min … max): 2.950 s … 3.204 s 10 runs Summary './manage.py fill_memcached_caches --skip-checks' ran 1.70 ± 0.07 times faster than './manage.py fill_memcached_caches' ``` --- scripts/lib/upgrade-zulip-stage-2 | 18 ++++++++++++++---- scripts/restart-server | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/scripts/lib/upgrade-zulip-stage-2 b/scripts/lib/upgrade-zulip-stage-2 index 499b133695..249cd99a52 100755 --- a/scripts/lib/upgrade-zulip-stage-2 +++ b/scripts/lib/upgrade-zulip-stage-2 @@ -320,6 +320,10 @@ else: preexec_fn=su_to_zulip, ) +# Perform system checks -- including database checks, so we don't need +# to do them when we do migrations, below. +subprocess.check_call(["./manage.py", "check", "--database", "default"], preexec_fn=su_to_zulip) + # Our next optimization is to check whether any migrations are needed # before we start the critical section of the restart. This saves # about 1s of downtime in a no-op upgrade. We omit this check if we @@ -330,7 +334,7 @@ if not IS_SERVER_UP: elif not args.skip_migrations: logging.info("Checking for needed migrations") migrations_output = subprocess.check_output( - ["./manage.py", "showmigrations"], preexec_fn=su_to_zulip, text=True + ["./manage.py", "showmigrations", "--skip-checks"], preexec_fn=su_to_zulip, text=True ) for ln in migrations_output.split("\n"): line_str = ln.strip() @@ -340,7 +344,9 @@ elif not args.skip_migrations: # If there are no migrations needed, we can fill the caches now, # instead of after they run. if not migrations_needed: - subprocess.check_call(["./manage.py", "fill_memcached_caches"], preexec_fn=su_to_zulip) + subprocess.check_call( + ["./manage.py", "fill_memcached_caches", "--skip-checks"], preexec_fn=su_to_zulip + ) # If we are planning on running puppet, we can pre-run it in --noop # mode and see if it will actually make any changes; if not, we can @@ -392,7 +398,9 @@ if migrations_needed: # quiesced state. shutdown_server() logging.info("Applying database migrations...") - subprocess.check_call(["./manage.py", "migrate", "--noinput"], preexec_fn=su_to_zulip) + subprocess.check_call( + ["./manage.py", "migrate", "--noinput", "--skip-checks"], preexec_fn=su_to_zulip + ) logging.info("Restarting Zulip...") start_args = [] @@ -412,7 +420,9 @@ logging.info("Upgrade complete!") if args.audit_fts_indexes: logging.info("Correcting full-text search indexes for updated dictionary files") logging.info("This may take a while but the server should work while it runs.") - subprocess.check_call(["./manage.py", "audit_fts_indexes"], preexec_fn=su_to_zulip) + subprocess.check_call( + ["./manage.py", "audit_fts_indexes", "--skip-checks"], preexec_fn=su_to_zulip + ) if not args.skip_purge_old_deployments: logging.info("Purging old deployments...") diff --git a/scripts/restart-server b/scripts/restart-server index 673e642dd9..44ce9ea32a 100755 --- a/scripts/restart-server +++ b/scripts/restart-server @@ -43,7 +43,7 @@ if pwd.getpwuid(os.getuid()).pw_name != "zulip": if args.fill_cache: logging.info("Filling memcached caches") - subprocess.check_call(["./manage.py", "fill_memcached_caches"]) + subprocess.check_call(["./manage.py", "fill_memcached_caches", "--skip-checks"]) current_symlink = os.path.join(DEPLOYMENTS_DIR, "current") last_symlink = os.path.join(DEPLOYMENTS_DIR, "last")