Skip to content

Commit

Permalink
[#24898] YSQL: Change extended query protocol flow with Connection Ma…
Browse files Browse the repository at this point in the history
…nager

Summary:
This patch changes the extended query protocol flow through connection manager to help facilitate correctness/consistency with batched mode of query execution.

Problem:
Connection Manager modifies extended query protocol-level communication between the client and server in order to support multiplexing of clients over a pool of server connections. This creates issues with the batched mode of execution owing to an incorrect order of packets being sent back to the client:
- Connection Manager would always immediately send back a self-constructed ParseComplete packet to the client as soon as it received a Parse packet from the client
- Connection Manager ignores ParseComplete packets sent by the server because it would conditionally send Parse packets from the client to the server

The solution largely does the following:
- Introduce a new flag `ysql_conn_mgr_optimized_extended_query_protocol` (bool val, default true)
  - if true, lessen correctness for scenarios where schema changes may be present between preparing statements, but increase performance
  - if false, extended query protocol will be supported with full correctness at the cost of performance
  - Note that either mode of execution ensures protocol-level communication correctness as well as consistency of batched mode of query execution.
- Always send client-sent Parse packets to the server (this used to only happen if the server attached had not already parsed the **query** - the protocol-level name would be ignored)
- Always forward server-sent ParseComplete packets back to the client (this used to always be ignored, Connection Manager would preemptively send back self-constructed ParseComplete packets to client)
- Change the statement name being sent to the server to not only depend upon the query body, but also include dependency on:
  - logical client ID (only if `ysql_conn_mgr_optimized_extended_query_protocol` is false)
  - client-provided protocol level name

The last change mentioned helps to deal with potential duplication issues as connection manager multiplexes multiple clients over multiple servers.

Some more intricate details regarding this solution:
- If we ever enter a scenario where Connection Manager routes the client to different servers between Parse and Bind phases, we will be forced to re-prepare the statement on the newer server selected - the Parse packets sent for these situations is a special packet specific to Connection Manager where we ask the server to not send back a ParseComplete packet to maintain protocol correctness between the client and server. (denoted by packet header 'p')
- In "optimized" extended query protocol mode, it is possible for different logical connections to prepare the same query with the same statement name on a common server - we ask the server to perform a no-op instead of parsing the duplicate statement, and return a ParseComplete packet to maintain client-side protocol correctness. (denoted by packet header 'n')

Misc fix included as a part of this diff:
- Provide handling for Describe packet for unnamed prepared statements (mimics what is done for Bind phase for unnamed prepared statements, can refer to D41078 for additional details)
Jira: DB-14026

Test Plan: Jenkins: enable connection manager, all tests

Reviewers: skumar, devansh.saxena, stiwary, mkumar, vpatibandla, jason

Reviewed By: devansh.saxena, mkumar, jason

Subscribers: smishra, yql

Differential Revision: https://phorge.dev.yugabyte.com/D41121
  • Loading branch information
rahulb-yb committed Jan 29, 2025
1 parent bf87067 commit f3b586d
Show file tree
Hide file tree
Showing 14 changed files with 283 additions and 75 deletions.
10 changes: 5 additions & 5 deletions src/odyssey/sources/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,10 @@ struct od_client {
int64_t logical_client_version;

/*
* This stores the last unnamed prepared statement, otherwise NULL.
* This stores the last unnamed prepared statement.
* Fields are NULL/0 if no such case.
*/
char* yb_unnamed_prep_stmt;
int yb_unnamed_prep_stmt_size;
kiwi_prepared_statement_t yb_unnamed_prep_stmt;
};

static const size_t OD_CLIENT_DEFAULT_HASHMAP_SZ = 420;
Expand Down Expand Up @@ -156,8 +156,7 @@ static inline void od_client_init(od_client_t *client)
client->yb_is_authenticating = false;
client->yb_external_client = NULL;
client->logical_client_version = 0;
client->yb_unnamed_prep_stmt = NULL;
client->yb_unnamed_prep_stmt_size = 0;
yb_prepared_statement_init(&client->yb_unnamed_prep_stmt);
}

static inline od_client_t *od_client_allocate(void)
Expand Down Expand Up @@ -188,6 +187,7 @@ static inline void od_client_free(od_client_t *client)
free(client->deploy_err);
client->deploy_err = NULL;
}
yb_prepared_statement_free(&client->yb_unnamed_prep_stmt);
free(client);
}

Expand Down
5 changes: 5 additions & 0 deletions src/odyssey/sources/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ void od_config_init(od_config_t *config)
config->coroutine_stack_size = 4;
config->hba_file = NULL;

/* YB */
config->yb_use_auth_backend = true;
config->yb_optimized_extended_query_protocol = true;
config->yb_enable_multi_route_pool = true;
// Same default as the value of ysql_max_connections.
config->yb_ysql_max_connections = 300;
Expand Down Expand Up @@ -334,6 +336,9 @@ void od_config_print(od_config_t *config, od_logger_t *logger)
od_log(logger, "config", NULL, NULL, "yb_use_auth_backend %s",
od_config_yes_no(config->yb_use_auth_backend));

od_log(logger, "config", NULL, NULL, "yb_optimized_extended_query_protocol %s",
od_config_yes_no(config->yb_optimized_extended_query_protocol));

od_log(logger, "config", NULL, NULL, "yb_enable_multi_route_pool %s",
od_config_yes_no(config->yb_enable_multi_route_pool));

Expand Down
1 change: 1 addition & 0 deletions src/odyssey/sources/config.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ struct od_config {
/* YB */
int yb_ysql_max_connections;
int yb_use_auth_backend;
int yb_optimized_extended_query_protocol;
int yb_enable_multi_route_pool;
};

Expand Down
11 changes: 11 additions & 0 deletions src/odyssey/sources/config_reader.c
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ typedef enum {

/* YB */
OD_YB_USE_AUTH_BACKEND,
OD_YB_OPTIMIZED_EXTENDED_QUERY_PROTOCOL,
OD_YB_ENABLE_MULTI_ROUTE_POOL,
OD_YB_YSQL_MAX_CONNECTIONS,
} od_lexeme_t;
Expand Down Expand Up @@ -324,6 +325,8 @@ static od_keyword_t od_config_keywords[] = {

/* YB */
od_keyword("yb_use_auth_backend", OD_YB_USE_AUTH_BACKEND),
od_keyword("yb_optimized_extended_query_protocol",
OD_YB_OPTIMIZED_EXTENDED_QUERY_PROTOCOL),
od_keyword("yb_enable_multi_route_pool", OD_YB_ENABLE_MULTI_ROUTE_POOL),
od_keyword("yb_ysql_max_connections", OD_YB_YSQL_MAX_CONNECTIONS),

Expand Down Expand Up @@ -2424,6 +2427,14 @@ static int od_config_reader_parse(od_config_reader_t *reader,
goto error;
}
continue;
/* yb_optimized_extended_query_protocol */
case OD_YB_OPTIMIZED_EXTENDED_QUERY_PROTOCOL:
if (!od_config_reader_yes_no(
reader,
&config->yb_optimized_extended_query_protocol)) {
goto error;
}
continue;
/* yb_enable_multi_route_pool */
case OD_YB_ENABLE_MULTI_ROUTE_POOL:
if (!od_config_reader_yes_no(reader,
Expand Down
Loading

0 comments on commit f3b586d

Please sign in to comment.