Skip to content

Add deep startup-performance diagnosis for pgAdmin desktop launch path#1

Draft
Copilot wants to merge 2 commits into
masterfrom
copilot/analyze-pgadmin-startup-performance
Draft

Add deep startup-performance diagnosis for pgAdmin desktop launch path#1
Copilot wants to merge 2 commits into
masterfrom
copilot/analyze-pgadmin-startup-performance

Conversation

Copy link
Copy Markdown

Copilot AI commented Mar 21, 2026

pgAdmin desktop startup can be disproportionately slow because heavy backend initialization completes before Electron can pass readiness checks. This PR adds a code-referenced diagnosis of the startup critical path, pinpoints concrete bottlenecks, and evaluates whether ~10x speedup is realistic and where limits/trade-offs exist.

  • What was added

    • New analysis document: docs/en_US/startup_performance_diagnosis.rst
    • Linked it from docs navigation in docs/en_US/index.rst
  • Bottlenecks identified (with exact code anchors)

    • Schema migration + table validation on startup
      web/pgadmin/__init__.py, web/pgadmin/setup/db_upgrade.py, web/pgadmin/setup/db_table_check.py
    • Eager module/blueprint loading during app creation
      web/pgadmin/submodules.py, web/pgadmin/__init__.py
    • Eager driver/auth registry loading
      web/pgadmin/utils/driver/registry.py, web/pgadmin/authenticate/registry.py
    • Serial config DB key fetches in startup path
      web/pgadmin/__init__.py
    • Electron readiness polling granularity (1s interval)
      runtime/src/js/pgadmin.js
    • Synchronous startup log writes in runtime path
      runtime/src/js/misc.js
  • Optimization feasibility and constraints

    • Assesses practical high-impact levers (lazy/incremental init, migration-path redesign, readiness signaling changes, batching serial DB reads).
    • Explicitly documents limiting factors and trade-offs (e.g., deferred-load first-hit latency, migration reliability complexity).
    • Concludes that near-10x is plausible in worst-case cold/upgrade scenarios, with lower gains on already-warm starts.
// Current runtime readiness gate (one major startup latency contributor)
pingIntervalID = setInterval(function () {
  pingServer().then(() => {
    launchPgAdminWindow();
  });
}, 1000);

💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

Co-authored-by: dev-hari-prasad <161151819+dev-hari-prasad@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dev-hari-prasad/pgadmin4/sessions/74114d00-7f5f-4015-9182-7838e71d5a32
Copilot AI changed the title [WIP] Investigate performance issues in pgAdmin application launch Add deep startup-performance diagnosis for pgAdmin desktop launch path Mar 21, 2026
Copilot AI requested a review from dev-hari-prasad March 21, 2026 18:14
dev-hari-prasad pushed a commit that referenced this pull request Jun 2, 2026
…st context

Two related issues that surfaced in BatchProcessTest:

1. ConnectionLocker.__enter__ accessed Flask session (via 'in' check)
   AFTER acquiring the lock. When called outside a request context,
   session access raises RuntimeError. Python's 'with' semantics skip
   __exit__ if __enter__ raises, so the lock leaked - any subsequent
   call to ConnectionLocker hung forever waiting on a lock that
   would never be released.

   Wrap session access in try/except RuntimeError so missing-request-
   context falls through cleanly and the lock is released normally
   on with-block exit. Also use .get() chains so partial session
   shapes do not raise KeyError.

2. The four batch_process unit tests (backup, import_export,
   maintenance, restore) used app_context() instead of
   test_request_context(). flask-babel's gettext() in
   BackupMessage.details() and similar code paths require a request
   context; ConnectionLocker.__enter__ also touches session as above.
   Switching to test_request_context('/') gives both the request
   binding they need.

Verified: tools.backup.tests.test_batch_process now runs 4/4 passing
(was 3 ERROR + 1 hang before fix #1; 3 FAIL before fix #2).

Both are pre-existing issues exposed by Python 3.14 / flask 3.1 /
flask-babel stricter context enforcement; not introduced by 9.15
CVE work.
dev-hari-prasad pushed a commit that referenced this pull request Jun 2, 2026
…lity test

Two related issues, both surfaced when running the regression suite
non-interactively:

1. user_info_server()'s while-not-validate retry loops had no upper
   bound. With a mocked or closed stdin (test mocks, EOF in CI/cron,
   typo'd PGADMIN_SETUP_EMAIL=''), the loop would call input()/pprompt()
   forever, printing 'Invalid email address. Please try again.' on
   every iteration. We saw this manifest as a 13.5 million-line
   25 GB log when test_no_email_deliverability hit the case.

   Cap each loop at MAX_PROMPT_ATTEMPTS=5 and raise RuntimeError with a
   pointer to PGADMIN_SETUP_EMAIL/PASSWORD env vars on exhaustion.

2. test_no_email_deliverability included pg@postgres.local in its
   "should be accepted with deliverability=False" data set. The .local
   suffix is in email_validator.SPECIAL_USE_DOMAIN_NAMES which fails
   syntactic validation regardless of the deliverability flag, so
   validate_email always rejected it - looping forever pre-fix #1.

   Set config.ALLOW_SPECIAL_EMAIL_DOMAINS = ['local'] in the test's
   try/finally block so .local is allowed for this scenario, restored
   afterward to avoid leaking state into other tests.

With both fixes in place, the test can be removed from the regression
runtests --exclude list.
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.

2 participants