Avoid reloading GUC variables on control connections #21516
Assignees
Labels
area/ecosystem
Label for all ecosystem related projects
jira-originated
kind/new-feature
This is a request for a completely new feature
priority/high
High Priority
Comments
yugabyte-ci
added
area/ecosystem
yugabyte-ci
changed the title
manav-yb
pushed a commit
that referenced
this issue
Jul 11, 2024
…tgres config file on it Summary: YB provides utility `yb-ts-cli` to modify the value of g-flags while cluster is running which changes the associated GUC variable value. It updates the `ysql_pg.conf` file with new GUC variable key, value pair and sends a SIGHUP signal to postmaster explicitly asking to reload the configuration file `ysql_pg.conf`. Postmaster forwards the SIGHUP signal to every child backend processes so that backend process also reloads new configuration file and sets the required GUC variable values on their connection. On setting a GUC variable a PARAMETER STATUS packet is forward to client which can be either due to reloading of entire config file or on setting a particular GUC variable using `SET`. In Ysql connection manager, control connections are used to authenticate the client and there can be some repercussion on reloading config file on a control connection. Therefore we defer to support reloading postgres config file with ysql connection manager into the later phase of the product. In this diff we are adopting a conservative approach of destroying the control connection for which config file needs to be reloaded. A client connection request would fail which tries using a control connection (for authentication) for which config file needs to be reloaded and control connection will be destroyed. Therefore client needs to make as many attempts as control connections are there for which config reload is required so that eventually a new control connection will be created with updated config file for authentication. Tests mentioned in the test plan were failing with ysql connection manager because on reloading the configuration file, control connection sends a PARAMETER STATUS packet (while setting all GUC variables present in file) to connection manager while authentication of a client which is not expected by design of the authentication in ysql connection manager. TODO: Add a dedicated test for GUC configuration reload, tracked by DB-12117. Jira: DB-10399 Test Plan: Jenkins: enable connection manager, all tests Ensure all below tests are running fine with ysql connection manager: # `org.yb.pgsql.TestSecureClusterLocalTServerHostName#testCertificateReload` # `org.yb.pgsql.TestPgReplicationSlot#testReplicationWithSpilledTransaction` # `org.yb.pgsql.TestToastFunction#testAdHocMemoryUsage` # `org.yb.pgsql.TestToastFunction#testBuildRelcacheInitFileMemoryUsage` # `org.yb.pgsql.TestToastFunction#testCatalogRefreshMemoryUsage` # `org.yb.pgsql.TestSecureCluster#testCertificateReload` Reviewers: asrinivasan, skumar Reviewed By: asrinivasan Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D36122
jasonyb
pushed a commit
that referenced
this issue
Jul 12, 2024
Summary: 9ca3158 [YNP] set the instanceName & nodeName during node registration with provider 1d2c37d [PLAT-14517] Add local provider test for create + delete db scoped DR replication. 2ff4f94 Fixes for 404 (#23180) 1f04b53 Early Access track (#23160) Excluded: 58b746e [#21516] YSQL: Destroy the control connection while reloading the postgres config file on it 55da508 [#23184] docdb: Add gflag to disable bootstrap intent filtering fae28ed YBM change log (#23186) b913931 [PLAT-12736] cleanup _index and universe.yaml Excluded: 968e831 [#22843] YSQL: Add YbNumCatalogCacheIdMisses to display PG catalog cache misses Test Plan: Jenkins: rebase: pg15-cherrypicks Reviewers: jason, tfoucher Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D36546
manav-yb
pushed a commit
that referenced
this issue
Jul 18, 2024
…ion while reloading the postgres config file on it
Summary:
YB provides utility `yb-ts-cli` to modify the value of g-flags while cluster is running which changes the associated GUC variable value. It updates the `ysql_pg.conf` file with new GUC variable key, value pair and sends a SIGHUP signal to postmaster explicitly asking to reload the configuration file `ysql_pg.conf`. Postmaster forwards the SIGHUP signal to every child backend processes so that backend process also reloads new configuration file and sets the required GUC variable values on their connection. On setting a GUC variable a PARAMETER STATUS packet is forward to client which can be either due to reloading of entire config file or on setting a particular GUC variable using `SET`.
In Ysql connection manager, control connections are used to authenticate the client and there can be some repercussion on reloading config file on a control connection. Therefore we defer to support reloading postgres config file with ysql connection manager into the later phase of the product. In this diff we are adopting a conservative approach of destroying the control connection for which config file needs to be reloaded. A client connection request would fail which tries using a control connection (for authentication) for which config file needs to be reloaded and control connection will be destroyed. Therefore client needs to make as many attempts as control connections are there for which config reload is required so that eventually a new control connection will be created with updated config file for authentication.
Tests mentioned in the test plan were failing with ysql connection manager because on reloading the configuration file, control connection sends a PARAMETER STATUS packet (while setting all GUC variables present in file) to connection manager while authentication of a client which is not expected by design of the authentication in ysql connection manager.
Following are the changes present in this diff:
- `BasePgSQLTest.java`
- Added a static integer variable (`MAX_ATTEMPTS_TO_DESTROY_CONTROL_CONN`) denoting maximum number of attempts. On cherry picking changes from master i was getting a merge conflict, due to changes done in this file as part of `6e887a9652a1` commit in pg15-cherrypick branch but not present in master. Resolved now.
TODO: Add a dedicated test for GUC configuration reload, tracked by DB-12117.
Jira: DB-10399
Test Plan:
Ensure all below tests are running fine with ysql connection manager:
# `org.yb.pgsql.TestSecureClusterLocalTServerHostName#testCertificateReload`
# `org.yb.pgsql.TestPgReplicationSlot#testReplicationWithSpilledTransaction`
# `org.yb.pgsql.TestToastFunction#testAdHocMemoryUsage`
# `org.yb.pgsql.TestToastFunction#testBuildRelcacheInitFileMemoryUsage`
# `org.yb.pgsql.TestToastFunction#testCatalogRefreshMemoryUsage`
# `org.yb.pgsql.TestSecureCluster#testCertificateReload`
Reviewers: asrinivasan, skumar, jason, tfoucher
Reviewed By: jason
Subscribers: yql
Differential Revision: https://phorge.dev.yugabyte.com/D36604
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jira Link: DB-10399
The text was updated successfully, but these errors were encountered: