Skip to content

Commit

Permalink
[#21438] YSQL: Forward errors around drop db/role as accurately as po…
Browse files Browse the repository at this point in the history
…ssible

Summary:
This diff changes the error forwarding in place when dealing with errors around database/user being dropped with Connection Manager.

Old behaviour:
- We would earlier construct an error message from the message/hint of the server's error (depending on invalid user or database, respectively) and forward to client, along with a hardcoded SQLState.
- In a scenario with multiple logical connections in a dropped/invalid pool we would use the information that if the current db/user is dropped, then construct an error from Connection Manager to be forwarded to the client, rather than relying on the error sent by the server

New behaviour:
- Forward the server error, including its detail and hint, if any, as it is, back to the client
- Despite of knowing that the currently used db/user is dropped, still forward the client query to the server, then forward this error transparently back to the user as in the previous point

Jira: DB-10320

Test Plan: Jenkins: enable connection manager, all tests

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

Reviewed By: devansh.saxena

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D41398
  • Loading branch information
rahulb-yb committed Jan 29, 2025
1 parent f3b586d commit 2575f61
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 22 deletions.
31 changes: 25 additions & 6 deletions src/odyssey/sources/backend.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@ void od_backend_error(od_server_t *server, char *context, char *data,
{
od_instance_t *instance = server->global->instance;
kiwi_fe_error_t error;
int detail_len = 0;
int hint_len = 0;

od_client_t *yb_server_client = server->client;
od_client_t *yb_external_client = (yb_server_client->yb_is_authenticating) ?
Expand All @@ -92,6 +94,7 @@ void od_backend_error(od_server_t *server, char *context, char *data,
if (error.detail) {
od_error(&instance->logger, context, yb_external_client, server,
"DETAIL: %s", error.detail);
detail_len = strlen(error.detail);
}

/* catch and store error to be forwarded later if we are in deploy phase */
Expand All @@ -104,6 +107,7 @@ void od_backend_error(od_server_t *server, char *context, char *data,
if (error.hint) {
od_error(&instance->logger, context, yb_external_client, server,
"HINT: %s", error.hint);
hint_len = strlen(error.hint);

if (strcmp(error.hint, "Database might have been dropped by another user") == 0)
{
Expand All @@ -113,10 +117,16 @@ void od_backend_error(od_server_t *server, char *context, char *data,
if (yb_external_client != NULL &&
((od_client_t *)yb_external_client)->type ==
OD_POOL_CLIENT_EXTERNAL)
od_frontend_error(
yb_external_client,
KIWI_CONNECTION_DOES_NOT_EXIST,
error.hint, od_io_error(&server->io));
{
machine_msg_t *msg;
msg = kiwi_be_write_error_as(NULL, error.severity, error.code,
error.detail, detail_len, error.hint,
hint_len, error.message, strlen(error.message));
/* YB: best-effort forward to client, already handling error */
if (msg == NULL)
return;
od_write(&yb_external_client->io, msg);
}
}
}

Expand All @@ -126,8 +136,17 @@ void od_backend_error(od_server_t *server, char *context, char *data,

if (yb_external_client != NULL &&
((od_client_t *)yb_external_client)->type == OD_POOL_CLIENT_EXTERNAL)
od_frontend_error(yb_external_client, KIWI_CONNECTION_DOES_NOT_EXIST, error.message);
}
{
machine_msg_t *msg;
msg = kiwi_be_write_error_as(NULL, error.severity, error.code,
error.detail, detail_len, error.hint,
hint_len, error.message, strlen(error.message));
/* YB: best-effort forward to client, already handling error */
if (msg == NULL)
return;
od_write(&yb_external_client->io, msg);
}
}
}

int od_backend_ready(od_server_t *server, char *data, uint32_t size)
Expand Down
17 changes: 1 addition & 16 deletions src/odyssey/sources/frontend.c
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,7 @@ od_frontend_attach(od_client_t *client, char *context,
continue;
}

/* YB: check auth failure status codes to update OID status */
if (yb_frontend_error_is_db_does_not_exist(client))
((od_route_t *)server->route)->yb_database_entry->status = YB_OID_DROPPED;
else if (yb_frontend_error_is_role_does_not_exist(client))
Expand Down Expand Up @@ -1977,22 +1978,6 @@ static od_frontend_status_t od_frontend_remote(od_client_t *client)

for (;;) {
for (;;) {
rc = yb_is_route_invalid(client->route);
if (rc == ROUTE_INVALID_DB_OID) {
od_frontend_fatal(
client, KIWI_CONNECTION_FAILURE,
"Database might have been dropped by another user");
status = OD_ECLIENT_READ;
break;
} else if (rc == ROUTE_INVALID_ROLE_OID) {
od_frontend_fatal(
client, KIWI_CONNECTION_FAILURE,
"invalid role OID: %d",
((od_route_t *)(client->route))->yb_user_entry->oid);
status = OD_ECLIENT_READ;
break;
}

if (od_should_drop_connection(client, server)) {
/* Odyssey is going to shut down or client conn is dropped
* due some idle timeout, we drop the connection */
Expand Down

0 comments on commit 2575f61

Please sign in to comment.