In connection manager, the SET configuration_parameter should be case insensitive #23478
Assignees
Labels
Comments
yugabyte-ci
added
jira-originated
kind/bug
vpatibandla-yb
added a commit
that referenced
this issue
Sep 11, 2024
…ty issue Summary: The SET session variables for a client connection are persisted in the vars field in client and server context. These session variables in postgres are not case sensitive yet in the connection manager layer they are being persisted in a case sensitive manner. The change here is to persist the session variables in connection manager in lower case and also the comparisons wherever needed are also done in lower case. The implication of this change is that when the session variables with the same name but different case sensitivity are used, we will parse it and interpret it only in lower case. For example, while we are running the pg regress tests, some environment variables are set such as `PGTZ`, `PGDATESTYLE` and `PGOPTIONS=-c intervalstyle=postgres_verbose`. These get translated to `timezone`, `datestyle` and options is parsed to `intervalstyle` correspondingly as part of the Startup packet parsing. This info is intercepted by connection manager and persisted in the client vars field. When these vars are sent to the backend postgres connection to set the client connection state, postgres returns them in a different case `timezone` as `TimeZone`, `datestyle` as `DateStyle` and `intervalstyle` as `IntervalStyle`. Connection manager when it receives the ParameterStatus packet parses them and persists in the client vars and server vars entries. This is resulting in the duplication of the same postgres GUC variables in different cases. This can lead to different failure scenarios. One of the failure is mentioned below: - As part of the postgres backend initialization, the following GUC options are set in the manner ``` SET intervalstyle=E'postgres_verbose';SET application_name=E'pg_regress/yb_pg_init';SET datestyle=E'Postgres, MDY';SET timezone=E'PST8PDT'; ``` - Now, during subsequent SET operation `SET client_min_messages = warning;`, the ParameterStatus messages are persisted in client vars. The `datestyle` is returned as `DateStyle` by postgres and is persisted the same way. So, now in client vars two options with the same key are present with different case sensitivity and the server vars has `DateStyle` . The client vars are ``` datestyle=Postgres, MDY DateStyle = Postgres, MDY ``` server vars are ``` DateStyle = Postgres, MDY ``` - Now, let us perform the next SET operation `SET DATESTYLE TO ISO;` . With this operation, the new client vars are ``` datestyle=Postgres, MDY DateStyle = ISO, MDY ``` The server vars are ``` DateStyle = ISO, MDY ``` - Now, if we try to perform any other operation using connection manager, the SET session variables command looks like as below ``` SET intervalstyle=E'postgres_verbose';SET application_name=E'pg_regress/yb_pg_orafce';SET datestyle=E'Postgres, MDY';SET timezone=E'PST8PDT';SET IntervalStyle=E'postgres_verbose';SET TimeZone=E'PST8PDT';SET client_min_messages=E'warning'; ``` In the above set statements, we will notice that `datestyle=E'Postgres, MDY'` even though the prior SET command was `SET DATESTYLE TO ISO;` . This is because if the client and server var for a certain option are the same in a case sensitive manner, then that SET command is not applied before the actual query is run. Yet, `datestyle=E'Postgres, MDY'` is being SET as the comparison is case sensitive. - To overcome this. I am persisting all the SET commands in lower case for client, server and also the comparisons wherever needed are done on lower case strings. So, all the SET option commands that are called before the Query will be called with lowercase option names. **Changes made** There are 3 places where the changes are made: # `kiwi_var_init` -> This method is called to initialize the kiwi_var struct. As part of this method, the name and name_len values are persisted. By the logic mentioned above, name would be converted to lowercase and persisted in `var->name.` # `yb_kiwi_var_push` -> This method is called to create a new entry in `kiwi_vars_t` struct. The new entry would be created with name in lowercase and the other fields are persisted as passed in the method. This method is primarily called by `kiwi_vars_update` which in turn is called while dealing with ParameterStatus or Startup packet messages. In both the cases as discussed above, we would like to persist the name in lowercase for correctness sake as explained in the example above. # `yb_kiwi_vars_get` -> This method is used to fetch the kiwi_var struct corresponding to the param name. As discussed above, the ParameterStatus response need not be in lower case and hence, the idea is to persist the entries in lower case and thereafter search them in lower case which will lead to consistent behaviour while dealing with session variables/GUC variables. Jira: DB-12395 Test Plan: Jenkins: test regex: .*TestPgRegress.*, enable connection manager The tests `org.yb.pgsql.TestPgRegressMisc#testPgRegressMiscSerial2` and `org.yb.pgsql.TestPgRegressPushdown#testPgRegressPushdown` in pg regress test this feature quite extensively. The issue was noticed when these tests failed and the fix is to address them. So, no additional tests are needed. Reviewers: asrinivasan, skumar, mkumar, rbarigidad, jason, stiwary Reviewed By: skumar Subscribers: svc_phabricator, jason, yql Differential Revision: https://phorge.dev.yugabyte.com/D37295
jasonyb
pushed a commit
that referenced
this issue
Sep 12, 2024
Summary: f5ad1fb [#23855] Fixing the calculation of automatic refactoring count. eed826d [#22519] YSQL: Move ExplicitRowLockBuffer class into separate file 99a27e6 [PLAT-14522] Taking yba-ctl backups with prometheus HTTPS e2a84b0 [PLAT-15044] Add preflight check for node addition in provider 7e40d89 [#20769] XCluster: Dynamically apply cdc_wal_retention_time_secs for XCluster aa41478 [#23858] build: fix ./yb_build.sh release --gcc11 bcf7f47 [PLAT-13910] Improve IAM credentials fetch logging and add retries 303a202 [#23778] xCluster: Remove the capability to rename xCluster replication groups Excluded: 58fd26e [#23652] YSQL: Fix TestPgRegressAnalyze.java timeout / database drop failure on TSAN build 31da65b [doc] yb_enable_bitmapscan flag (#23854) 798db14 [#20335] DocDB: Use MonoClock for write query metric 80779d8 [#23860] xCluster: Add automatic ddl mode proto fields afd763d Revert "Revert "[PLAT-14786] Add support to node_agent install to use bind ip and node_external_fqdn"" e86951a [#23841] docdb: Disable stack trace tracking in TSAN builds d600608 [PLAT-15244] Fix schedule not getting updated on edit schedule API call 3c0df09 [PLAT-15214][PLAT-15232]YBC version upgrade to 2.2.0.0-b6 and enable YBC verbose by default aa7372e [#23478] YSQL: fix connection manager session variable case sensitivity issue 0d53558 [PLAT-14810][PLAT-14811][YBA CLI] Support adding and editing EIT configurations ffa537e [PLAT-10706][dr] Support retry-ability of failover and switchover Excluded: 5ae4558 [#23578] YSQL: Add HELP and TYPE to :13000/prometheus-metrics 3aa7459 [PLAT-15016] Handle gflag_group changes for ENHANCED_PG_COMPATIBILITY group in 2024.1.3 c89356c [PLAT-10592][YBA] Changes to support global tserver/master service in K8s 7fc3b76 [#3893] YCQL: Align 2 system_schema.* tables with Cassandra da6274e [23646] Test Stability: Fix PgMiniTest.FollowerReads 5fa6dc9 [PLAT-15180][Platform][UI][PITR]Create Restore Backup modal be0d1d1 [PLAT-15247][Platform][Backup]Create Backup scheduled policy List Test Plan: Jenkins: rebase: pg15-cherrypicks Reviewers: jason, tfoucher Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D37981
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Jira Link: DB-12395
The text was updated successfully, but these errors were encountered: