Skip to content

[*] set lock_timeout via connection RuntimeParams instead of per-query transaction#1067

Merged
pashagolub merged 3 commits intocybertec-postgresql:masterfrom
NikolayS:claude/postgres-lock-timeout-config-01CV6RHcoPstQMGhREGnNE8r
Dec 8, 2025
Merged

[*] set lock_timeout via connection RuntimeParams instead of per-query transaction#1067
pashagolub merged 3 commits intocybertec-postgresql:masterfrom
NikolayS:claude/postgres-lock-timeout-config-01CV6RHcoPstQMGhREGnNE8r

Conversation

@NikolayS
Copy link
Copy Markdown
Contributor

@NikolayS NikolayS commented Dec 4, 2025

Reduces metric queries from 4 roundtrips (BEGIN/SET/query/COMMIT) to 1 by setting lock_timeout at connection level via pgx RuntimeParams.

  • Add --conn-lock-timeout option (default: 100ms, env: PW_CONN_LOCK_TIMEOUT)
  • Set lock_timeout in RuntimeParams for PostgreSQL sources on connect
  • Simplify QueryMeasurements by removing transaction wrapper
  • Set to "0" to disable lock_timeout

Ref: https://brandur.org/fragments/postgres-parameters

@pashagolub pashagolub self-assigned this Dec 4, 2025
@pashagolub pashagolub added metrics Metrics related issues refactoring Something done as it should've been done from the start labels Dec 4, 2025
@pashagolub pashagolub force-pushed the claude/postgres-lock-timeout-config-01CV6RHcoPstQMGhREGnNE8r branch from 03456f7 to 2fe21ee Compare December 4, 2025 13:31
@pashagolub
Copy link
Copy Markdown
Collaborator

Thanks! Can you ask Claude to fix tests or should I? :-)

@NikolayS
Copy link
Copy Markdown
Contributor Author

NikolayS commented Dec 4, 2025

Thanks! Can you ask Claude to fix tests or should I? :-)

@pashagolub attempted in 56d368f

@coveralls
Copy link
Copy Markdown

coveralls commented Dec 4, 2025

Pull Request Test Coverage Report for Build 20029932099

Details

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 73.988%

Files with Coverage Reduction New Missed Lines %
internal/reaper/database.go 1 76.66%
Totals Coverage Status
Change from base Build 19968648019: 0.02%
Covered Lines: 3766
Relevant Lines: 5090

💛 - Coveralls

claude and others added 3 commits December 8, 2025 14:33
…y transaction

Reduces metric queries from 4 roundtrips (BEGIN/SET/query/COMMIT) to 1 by
setting lock_timeout at connection level via pgx RuntimeParams.

- Add --conn-lock-timeout option (default: 100ms, env: PW_CONN_LOCK_TIMEOUT)
- Set lock_timeout in RuntimeParams for PostgreSQL sources on connect
- Simplify QueryMeasurements by removing transaction wrapper
- Set to "0" to disable lock_timeout

Ref: https://brandur.org/fragments/postgres-parameters
Tests no longer expect transaction wrapping (BEGIN/SET/COMMIT) since
lock_timeout is now set via RuntimeParams at connection time.
User may control connection options through connection string, e.g.
`--source=postgresql://foo:baz@bar/db?lock_timeout=100`
@pashagolub pashagolub force-pushed the claude/postgres-lock-timeout-config-01CV6RHcoPstQMGhREGnNE8r branch from fdc08e8 to b97d7f3 Compare December 8, 2025 13:34
@pashagolub pashagolub merged commit b38f22e into cybertec-postgresql:master Dec 8, 2025
5 checks passed
pashagolub added a commit that referenced this pull request Dec 8, 2025
0xgouda added a commit that referenced this pull request Dec 9, 2025
* Move `sources` package helper functions to `testutil`.

* Move all helpers in `webserver` package to `testutil`.

* Add tests for `Mock{Sources,Metrics}ReaderWriter` to increase test coverage.

* Use `testutil` instead of manual container setup.

* Use testutil `helpers` instead of manually creating mocks/containers.

* Remove `TestSetupRPCServers()`

The setup function should be invoked once across all tests
because it setups a global port listener,
otherwise running tests parallely
may fail with `address already in use` error.

* Add tests to increase test coverage.

* remove `testutil.ExpectQuery`, overruled by #1067

* move `createTestSourceConn()` back to reaper

* Set etcd container version in `EtcdVersion` const.

It maybe useful for logging.

* Revert moving `mockFs` to `testutils`.

* Revert signature change in `createTestSourceConn()`

* Fix line spacing.

---------

Co-authored-by: Pavlo Golub <pavlo.golub@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

metrics Metrics related issues refactoring Something done as it should've been done from the start

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants