Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[YSQL][ASH] Instrument root_request_id at a higher level #22431

Closed
1 task done
abhinab-yb opened this issue May 17, 2024 · 0 comments
Closed
1 task done

[YSQL][ASH] Instrument root_request_id at a higher level #22431

abhinab-yb opened this issue May 17, 2024 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@abhinab-yb
Copy link
Contributor

abhinab-yb commented May 17, 2024

Jira Link: DB-11338

Description

The current implementation instruments the root_request_id at the post parse and executor hooks. This means that in case of multi-statement queries, we will get different root_request_ids. But for a single request from a client, we should have only one root_request_id.

Issue Type

kind/bug

Warning: Please confirm that this issue does not contain any sensitive information

  • I confirm this issue does not contain any sensitive information.
@abhinab-yb abhinab-yb added the area/ysql Yugabyte SQL (YSQL) label May 17, 2024
@abhinab-yb abhinab-yb self-assigned this May 17, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue labels May 17, 2024
abhinab-yb added a commit that referenced this issue May 20, 2024
Summary:
According to the postgres protocol (https://www.postgresql.org/docs/current/protocol-flow.html),
the server will always respond with a ReadyForQuery message when it finishes processing a request
to let the client know that it's ready to process a new query.

We set the metadata after the first input message after a ReadyForQuery message was sent.
We unset this metadata when we are just about the send a ReadyForQuery message to the client.
This way we don't have to worry about all the individual cases, and whether this is a simple query
or an extended query.

This means that for every incoming request from the client, we set the root request id once.
If there is a multi-statement query request, all the queries will have the same root request id
because all of them came in a single request from the client. Note that the query id will not
be same in this case, it will keep changing as the server keeps processing each individual
query.

Only in case the client sends a termination request, or the connection is terminated
unexpectedly, we set the metadata but never unset it. But it should be fine, since the
backend process is killed just after that, that means we cannot fetch samples for this
backend.

```
bin/ysqlsh -c "create table if not exists a(id int); insert into a select i from generate_series(1, 1000) as i;"

yugabyte=# SELECT
yugabyte-#     SUBSTRING(query, 1, 50) AS query,
yugabyte-#     root_request_id,
yugabyte-#     wait_event_component,
yugabyte-#     wait_event,
yugabyte-#     wait_event_type,
yugabyte-#     COUNT(*)
yugabyte-# FROM
yugabyte-#     yb_active_session_history
yugabyte-# JOIN
yugabyte-#     pg_stat_statements
yugabyte-# ON
yugabyte-#     query_id = queryid
yugabyte-# WHERE
yugabyte-#     sample_time >= current_timestamp - interval '20 minutes'
yugabyte-# GROUP BY
yugabyte-#     query,
yugabyte-#     root_request_id,
yugabyte-#     wait_event_component,
yugabyte-#     wait_event,
yugabyte-#     wait_event_type
yugabyte-# ORDER BY
yugabyte-#     root_request_id,
yugabyte-#     query,
yugabyte-#     wait_event_component,
yugabyte-#     wait_event_type;
                       query                        |           root_request_id            | wait_event_component |        wait_event        | wait_event_type | count
----------------------------------------------------+--------------------------------------+----------------------+--------------------------+-----------------+-------
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | OnCpu_Active             | Cpu             |     2
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | YBClient_WaitingOnDocDB  | Network         |    13
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | QueryProcessing          | Cpu             |    11
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | CatalogRead              | Network         |    13
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | StorageFlush             | Network         |     1
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | Raft_ApplyingEdits       | Cpu             |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | OnCpu_Passive            | Cpu             |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | YBClient_LookingUpTablet | Network         |     1
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | YBClient_WaitingOnDocDB  | Network         |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | QueryProcessing          | Cpu             |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | StorageFlush             | Network         |     3
```
Jira: DB-11338

Test Plan: Jenkins

Reviewers: jason

Reviewed By: jason

Subscribers: hbhanawat, amitanand, yql

Differential Revision: https://phorge.dev.yugabyte.com/D35126
svarnau pushed a commit that referenced this issue May 25, 2024
Summary:
According to the postgres protocol (https://www.postgresql.org/docs/current/protocol-flow.html),
the server will always respond with a ReadyForQuery message when it finishes processing a request
to let the client know that it's ready to process a new query.

We set the metadata after the first input message after a ReadyForQuery message was sent.
We unset this metadata when we are just about the send a ReadyForQuery message to the client.
This way we don't have to worry about all the individual cases, and whether this is a simple query
or an extended query.

This means that for every incoming request from the client, we set the root request id once.
If there is a multi-statement query request, all the queries will have the same root request id
because all of them came in a single request from the client. Note that the query id will not
be same in this case, it will keep changing as the server keeps processing each individual
query.

Only in case the client sends a termination request, or the connection is terminated
unexpectedly, we set the metadata but never unset it. But it should be fine, since the
backend process is killed just after that, that means we cannot fetch samples for this
backend.

```
bin/ysqlsh -c "create table if not exists a(id int); insert into a select i from generate_series(1, 1000) as i;"

yugabyte=# SELECT
yugabyte-#     SUBSTRING(query, 1, 50) AS query,
yugabyte-#     root_request_id,
yugabyte-#     wait_event_component,
yugabyte-#     wait_event,
yugabyte-#     wait_event_type,
yugabyte-#     COUNT(*)
yugabyte-# FROM
yugabyte-#     yb_active_session_history
yugabyte-# JOIN
yugabyte-#     pg_stat_statements
yugabyte-# ON
yugabyte-#     query_id = queryid
yugabyte-# WHERE
yugabyte-#     sample_time >= current_timestamp - interval '20 minutes'
yugabyte-# GROUP BY
yugabyte-#     query,
yugabyte-#     root_request_id,
yugabyte-#     wait_event_component,
yugabyte-#     wait_event,
yugabyte-#     wait_event_type
yugabyte-# ORDER BY
yugabyte-#     root_request_id,
yugabyte-#     query,
yugabyte-#     wait_event_component,
yugabyte-#     wait_event_type;
                       query                        |           root_request_id            | wait_event_component |        wait_event        | wait_event_type | count
----------------------------------------------------+--------------------------------------+----------------------+--------------------------+-----------------+-------
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | OnCpu_Active             | Cpu             |     2
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | YBClient_WaitingOnDocDB  | Network         |    13
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | QueryProcessing          | Cpu             |    11
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | CatalogRead              | Network         |    13
 create table if not exists a(id int)               | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | StorageFlush             | Network         |     1
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | Raft_ApplyingEdits       | Cpu             |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | OnCpu_Passive            | Cpu             |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | YBClient_LookingUpTablet | Network         |     1
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | TServer              | YBClient_WaitingOnDocDB  | Network         |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | QueryProcessing          | Cpu             |     2
 insert into a select i from generate_series($1, $2 | 0e5eed5a-a324-0ffb-d53d-58b6d01c48a7 | YSQL                 | StorageFlush             | Network         |     3
```
Jira: DB-11338

Test Plan: Jenkins

Reviewers: jason

Reviewed By: jason

Subscribers: hbhanawat, amitanand, yql

Differential Revision: https://phorge.dev.yugabyte.com/D35126
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue
Projects
None yet
Development

No branches or pull requests

2 participants