[DB] use queue pool for async db engine

[rohansonecha]

Addresses #7829 (review). The main benefit for using QueuePools as compared to NullPools is that connection pooling allows maintaining long running connections in memory for efficient re-use (ref).

Tested with local postgres (NullPools), initialized the sync engine as well as the async engine. Sync engine is initialized by most normal operations like sky launch, sky status, etc. Async engine is used by provision logs for checking the cluster status, so I ran provision logs during cluster launch to verify proper async engine initialization.

I ran the same tests described above with a remote api server and a beefy cloud sql instance to force using QueuePools and veriifed that both sync and async engine initialization works as expected.

Tested (run the relevant ones):

• Code formatting: install pre-commit (auto-check on commit) or bash format.sh
• Any manual or new tests for this PR (please specify below)
• All smoke tests: /smoke-test (CI) or pytest tests/test_smoke.py (local)
• Relevant individual tests: /smoke-test -k test_name (CI) or pytest tests/test_smoke.py::test_name (local)
• Backward compatibility: /quicktest-core (CI) or pytest tests/smoke_tests/test_backward_compat.py (local)

[rohansonecha]

/quicktest-core
/smoke-test
/smoke-test --postgres

[rohansonecha]

/smoke-test -k test_managed_jobs_basic

[rohansonecha]

/smoke-test --postgres -k test_managed_jobs_logs_gc

[rohansonecha]

/smoke-test --postgres -k test_nonexistent_bucket
/smoke-test --postgres -k test_upload_to_existing_bucket

[rohansonecha]

/smoke-test -k test_managed_jobs_basic
/smoke-test --postgres -k test_managed_jobs_logs_gc
/smoke-test --postgres -k test_nonexistent_bucket
/smoke-test --postgres -k test_upload_to_existing_bucket

[Michaelvll]

Can we also mention what is the major benefits we are getting from this?

[rohansonecha]

/smoke-test --postgres -k test_managed_jobs_logs_gc
/smoke-test --postgres -k test_nonexistent_bucket
/smoke-test --postgres -k test_upload_to_existing_bucket

[rohansonecha]

/smoke-test --postgres -k test_managed_jobs_logs_gc
/smoke-test --postgres -k test_nonexistent_bucket
/smoke-test --postgres -k test_upload_to_existing_bucket

[rohansonecha]

Can we also mention what is the major benefits we are getting from this?

Updated the PR description

[SeungjinYang]

Thanks @rohansonecha! Couple of minor comments, but this should be a meaningful improvement to DB efficiency

[SeungjinYang]

I do understand you don't need a separate engine cache for async and sync engines because their conn str would be different, but I actually suggest separating them out on a readability argument (so there's no second guessing involved at all)

[SeungjinYang]

Eh, I thought this would be easy enough because I thought all cases where code gets / sets from cache is if / elsed on sync / async anyway, but that's not the case. I'm fine with you adding a comment somewhere in code on why it's safe to use the same cache across sync and async engines.

[rohansonecha]

Discussed offline, but I think adding a separate engine cache would clutter the logic in this code block. I added a comment to improve the clarity and future maintainability of this code path.

[rohansonecha]

/quicktest-core
/smoke-test -k basic
/smoke-test --postgres -k basic