Skip to content
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

Fix autocommit footguns in performance tests #9735

Merged
merged 1 commit into from
Nov 12, 2024
Merged

Conversation

tristan957
Copy link
Member

@tristan957 tristan957 commented Nov 12, 2024

psycopg2 has the following warning related to autocommit:

By default, any query execution, including a simple SELECT will start
a transaction: for long-running programs, if no further action is
taken, the session will remain “idle in transaction”, an undesirable
condition for several reasons (locks are held by the session, tables
bloat…). For long lived scripts, either ensure to terminate a
transaction as soon as possible or use an autocommit connection.

In the 2.9 release notes, psycopg2 also made the following change:

with connection starts a transaction on autocommit transactions too

Some of these connections are indeed long-lived, so we were retaining tons of WAL on the endpoints because we had a transaction pinned in the past.

Link: https://www.psycopg.org/docs/news.html#what-s-new-in-psycopg-2-9
Link: psycopg/psycopg2#941

psycopg2 has the following warning related to autocommit:

> By default, any query execution, including a simple SELECT will start
> a transaction: for long-running programs, if no further action is
> taken, the session will remain “idle in transaction”, an undesirable
> condition for several reasons (locks are held by the session, tables
> bloat…). For long lived scripts, either ensure to terminate a
> transaction as soon as possible or use an autocommit connection.

Some of these connections are indeed long-lived, so we were retaining
tons of WAL on the endpoints because we had a transaction pinned in the
past.

Signed-off-by: Tristan Partin <tristan@neon.tech>
@tristan957 tristan957 requested a review from bayandin November 12, 2024 20:28
Copy link
Member

@bayandin bayandin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

Copy link

5382 tests run: 5162 passed, 0 failed, 220 skipped (full report)


Flaky tests (2)

Postgres 17

Code coverage* (full report)

  • functions: 31.8% (7891 of 24834 functions)
  • lines: 49.5% (62458 of 126257 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
2bedbbd at 2024-11-12T21:10:30.694Z :recycle:

@tristan957 tristan957 merged commit d8f5d43 into main Nov 12, 2024
82 checks passed
@tristan957 tristan957 deleted the tristan957/jsonnet branch November 12, 2024 21:48
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.

3 participants