Repository navigation
perf: cut avoidable SQL round trips from every endpoint - #63
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Database round trips dominate request time once Postgres is across a network. This counts the statements each endpoint sends and removes the ones advanced-alchemy adds without need, following fastapi-sqlalchemy-template#95 and litestar-sqlalchemy-template#72.
Counts
Measured with a
before_cursor_executelistener on the test connection, savepoints excluded. Round trips (RT) add BEGIN and COMMIT perTransaction, since the asyncpg adapter sends each as its own round trip. Authenticated requests also run the revocation check on their own checkout.Login, message listing and message replays are unchanged apart from the cheaper revocation check.
tests/api/test_statement_counts.pypins every count above.Changes
CheckTokenRevokedUseCasegetsDatabase.autocommit_transaction, adb_retry.Transactionopened atAUTOCOMMIT. Its single SELECT then goes out without BEGIN or COMMIT, so every authenticated request saves two round trips. The query is still inside theTransactionblock, so ADR 0010's recovery still applies; the pooled connection-loss test covers it. The test fixture overridesDatabase.autocommit_isolation_leveltoNone, because the asyncpg adapter rolls back an open transaction when its isolation level changes, and that would drop the outer test transaction.test_check_token_revoked.pychecks against a real engine that asyncpg reports no open transaction during the query. The lost-connection-in-session case no longer includes the revocation check: that state cannot happen for it, since it always starts on a fresh session from its own child container, and with an existing connection SQLAlchemy ignores the isolation option and warns.create(..., auto_refresh=False)for users and group chats drops the refresh SELECT, since the remaining columns have Python-side defaults. Members go in as oneINSERT ... RETURNING, and the returned rows are attached withset_committed_value, so a new chat is not read back. A direct-chat replay loads members withjoinedloadanduniquify=True.GET /chats/{id}/loads the chat with its members in one query and checks membership in Python. A missing chat still returns 403. The listing loadslast_messagewithjoinedload.update(item_id=...)read the row twice and refreshed it after the flush. It is replaced by plainUPDATEstatements, withRETURNINGfor edits. Updates tochatssetupdated_atthemselves, because the audit listener does not run for core updates.UPDATE chats SET last_message_id = (newest active message) WHERE id = :chat AND last_message_id = :deleted. That removes the chat read and the separate newest-message query, and the check and the write happen in one statement.EXISTSmembership flag. Mark-read fetches the member row together with anEXISTSfor the target message. Error order is unchanged: 404, then 403 for membership, then 403 for authorship; for mark-read, 403 then 400.DELETEinstead ofdelete_where, which selected the rows first.Testing
just test: 154 passed, coverage 100%.just lintis clean. The first commit adds the statement-count tests, and they fail on main.