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

Reconnect when server role changed #1053

Merged
merged 1 commit into from
Oct 9, 2023

Conversation

ermakov-oleg
Copy link
Contributor

Now when using target_session_attrs, for example "primary" and if the leader in the cluster changes - the connections continue to open to the replica.
Since the server role check is performed only at the first connection and all information for further connections is cached in Pool._working_*.

This PR proposes to add server role checking on every connection when using target_session_attrs and if the server role does not match the one specified in target_session_attrs, clear the Pool._working_* cache.

The impact on performance is minimal and will only be if target_session_attrs is used.
Without these changes target_session_attrs becomes useless, as it really works only at application startup.

@ermakov-oleg
Copy link
Contributor Author

Hi @elprans!
I fixed the errors I had on CI, can you run Regression Tests again?

@ermakov-oleg
Copy link
Contributor Author

Hello @elprans,

I wanted to discuss the proposed change and gain some insights into your perspective on it. Specifically, I'm interested in understanding how you feel about this modification and what might be preventing us from merging it.

Your feedback would be greatly appreciated, as it will help us align our efforts and make any necessary adjustments to move forward with the change. Please let me know your thoughts and any concerns you might have.

Thank you in advance for your time and assistance.

@elprans elprans force-pushed the reconnect-on-role-changed branch 3 times, most recently from f89c95c to 7c93292 Compare October 9, 2023 04:26
Currently, `asyncpt.Pool` will cache various aspects of the connection,
like the selected host and connection parameters in an attempt to make
subsequent connection attempts somewhat faster.  This behavior is
dubious because server host availability and role may change, such as
when a primary becomes a standby and vice-versa or when a host becomes
unavailable permanently, but another host from the DSN can be picked up.
Just remove it.
@elprans
Copy link
Member

elprans commented Oct 9, 2023

@ermakov-oleg, thanks for the PR! I think a better approach would be to just rip the whole _working_ stuff out and do a normal connection path every time, so I did just that.

@elprans elprans merged commit 4ddb039 into MagicStack:master Oct 9, 2023
33 checks passed
elprans added a commit that referenced this pull request Nov 5, 2023
Minor fixes and improvements.

Improvements
============

* Python 3.12 and PostgreSQL 16 support (#1084)
  (by @elprans in deea86c)

* Add support for tuple-format custom codecs on composite types (#1061)
  (by @elprans in 922fcd1)

* Support `target_session_attrs` in URL format, add tests (#1073)
  (by @elprans in 7cb4e70)

* Infinity numeric support (#1067)
  (by @krokoziabla in 0c3bf60 for #1020)

* Add support for the `WHERE` clause in `copy_to` methods (#941)
  (by @kaylynn234 in b7ffab6)

* Add query logging callbacks and context manager (#1043)
  (by @dcwatson in b2697ff)

Fixes
=====

* When prepared statements are disabled, avoid relying on them harder (#1065)
  (by @elprans in cbf64e1)

* Handle environments with HOME set to a not-a-directory (#1063)
  (by @elprans in af922bc)

* Fix handling of non-ASCII passwords (#1062)
  (by @elprans in 89d5bd0)

* Disable JIT while doing type introspection (#1082)
  (by @elprans in f21ebf6)

* Remove connection parameter caching in `Pool` (#1053)
  (by @ermakov-oleg in 4ddb039)

* Switch to Python 3.12-style `wait_for` (#1086)
  (by @elprans in 4bdd8a7)

* Update automatic PostGIS type conversion for Shapely 2.0 (#1085)
  (by @ChimneySwift in 8b45beb)

* Use the `timeout` context manager in the connection path (#1087)
  (by @elprans in 313b2b2)

* Small fix for documentation on using SSL in Connection (#995)
  (by @ScottFred in ccc7baf)

* Use cleanup_ctx in pool usage doc (#878)
  (by @ir4y in 70c8bd8)

* Close cursor portals once the iterator is exhausted (#1088)
  (by @elprans in ca9f03b)

* Cut BaseProtocol circular reference on close. (#1049)
  (by @pteromys in 93a6f79)

* Allow passing hosts as tuples to `connect()` (in addition to lists) (#1021)
  (by @lezram in d7faaff)

Other
=====

* Drop support for Python 3.7 (#1064)
  (by @bryanforbes in 87ab143)
@elprans elprans mentioned this pull request Nov 5, 2023
elprans added a commit that referenced this pull request Nov 5, 2023
Minor fixes and improvements.

Improvements
============

* Python 3.12 and PostgreSQL 16 support (#1084)
  (by @elprans in deea86c)

* Add support for tuple-format custom codecs on composite types (#1061)
  (by @elprans in 922fcd1)

* Support `target_session_attrs` in URL format, add tests (#1073)
  (by @elprans in 7cb4e70)

* Infinity numeric support (#1067)
  (by @krokoziabla in 0c3bf60 for #1020)

* Add support for the `WHERE` clause in `copy_to` methods (#941)
  (by @kaylynn234 in b7ffab6)

* Add query logging callbacks and context manager (#1043)
  (by @dcwatson in b2697ff)

Fixes
=====

* When prepared statements are disabled, avoid relying on them harder (#1065)
  (by @elprans in cbf64e1)

* Handle environments with HOME set to a not-a-directory (#1063)
  (by @elprans in af922bc)

* Fix handling of non-ASCII passwords (#1062)
  (by @elprans in 89d5bd0)

* Disable JIT while doing type introspection (#1082)
  (by @elprans in f21ebf6)

* Remove connection parameter caching in `Pool` (#1053)
  (by @ermakov-oleg in 4ddb039)

* Switch to Python 3.12-style `wait_for` (#1086)
  (by @elprans in 4bdd8a7)

* Update automatic PostGIS type conversion for Shapely 2.0 (#1085)
  (by @ChimneySwift in 8b45beb)

* Use the `timeout` context manager in the connection path (#1087)
  (by @elprans in 313b2b2)

* Small fix for documentation on using SSL in Connection (#995)
  (by @ScottFred in ccc7baf)

* Use cleanup_ctx in pool usage doc (#878)
  (by @ir4y in 70c8bd8)

* Close cursor portals once the iterator is exhausted (#1088)
  (by @elprans in ca9f03b)

* Cut BaseProtocol circular reference on close. (#1049)
  (by @pteromys in 93a6f79)

* Allow passing hosts as tuples to `connect()` (in addition to lists) (#1021)
  (by @lezram in d7faaff)

Other
=====

* Drop support for Python 3.7 (#1064)
  (by @bryanforbes in 87ab143)
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