Skip to content

Commit

Permalink
[#22816] YSQL: Bug fixes for replication connections in ysql connecti…
Browse files Browse the repository at this point in the history
…on manager

Summary:
Odyssey has the support for replication connections and makes them sticky through out the lifetime of logical connection.

There are 3 bugs discussed below in ysql connection manager for replication connections which got exposed on running `TestPgReplicationSlot` test.

  # Once replication connection disconnects, the corresponding walsender process instead of getting shutdown, it goes back to the pool to get attached to next replication connection. This bug was recently fixed as [[ yandex/odyssey@ccd4dbf | commit ]] in upstream odyssey. Doing the same change in ysql connection manager as well.

  # In ysql connection manager, the session parameters received in the startup packet from the driver are never SET for the replication connections . Therefore it lead to different behaviour for replication connections with and without connection manager for the client. It is fixed by removing the special check for replication connections which doesn't let session variables to SET.

  # Avoid resetting SESSION variables for replication connections because replication connections are sticky for the life time of the connection in odyssey and the server (Walsender) process gets expired when replication connection disconnects so need to reset the GUC variables. Also as upstream odyssey provides full support for replication connection and resetting of GUC variable at end of transaction is introduced by ysql connection manager therefore matching the same behaviour.

Jira: DB-11713

Test Plan: Jenkins: enable connection manager, test regex: .*TestPgReplicationSlot.*

Reviewers: skumar, nkumar, asrinivasan

Reviewed By: asrinivasan

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D35739
  • Loading branch information
Manav Kumar committed Jun 28, 2024
1 parent 58c8d4e commit a70681d
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 7 deletions.
4 changes: 0 additions & 4 deletions src/odyssey/sources/deploy.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ int od_deploy(od_client_t *client, char *context)
od_server_t *server = client->server;
od_route_t *route = client->route;

if (route->id.physical_rep || route->id.logical_rep) {
return 0;
}

#if YB_ENABLED == TRUE
/* compare and set options which are differs from server */
int query_count;
Expand Down
2 changes: 1 addition & 1 deletion src/odyssey/sources/reset.c
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ int od_reset(od_server_t *server)
goto error;
}

/* TODO: Add an if block */
if (!route->id.logical_rep)
{
char query_reset[] = "RESET ROLE;SET SESSION AUTHORIZATION DEFAULT;RESET ALL";
rc = od_backend_query(server, "reset-resetall", query_reset,
Expand Down
17 changes: 15 additions & 2 deletions src/odyssey/sources/router.c
Original file line number Diff line number Diff line change
Expand Up @@ -724,9 +724,22 @@ void od_router_detach(od_router_t *router, od_client_t *client)
* As of D26669, these queries are:
* a. Creating TEMP TABLES.
* b. Use of WITH HOLD CURSORS.
* c. Client connection is a logical or physical replication connection
*/
if (od_likely(!server->offline) && server->yb_sticky_connection == false) {
od_pg_server_pool_set(&route->server_pool, server, OD_SERVER_IDLE);
if (od_likely(!server->offline) && !server->yb_sticky_connection) {
od_instance_t *instance = server->global->instance;
if (route->id.physical_rep || route->id.logical_rep) {
od_debug(&instance->logger, "expire-replication", NULL,
server, "closing replication connection");
server->route = NULL;
od_backend_close_connection(server);
od_pg_server_pool_set(&route->server_pool, server,
OD_SERVER_UNDEF);
od_backend_close(server);
} else {
od_pg_server_pool_set(&route->server_pool, server,
OD_SERVER_IDLE);
}
} else {
od_instance_t *instance = server->global->instance;
od_debug(&instance->logger, "expire", NULL, server,
Expand Down

0 comments on commit a70681d

Please sign in to comment.