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][Query Diagnostics] Fix ASAN test failures caused by inconsistency in fetching pg_stat_statements normalised query string #23788

Closed
1 task done
IshanChhangani opened this issue Sep 4, 2024 · 0 comments
Assignees
Labels
area/ysql Yugabyte SQL (YSQL) kind/bug This issue is a bug priority/medium Medium priority issue

Comments

@IshanChhangani
Copy link
Contributor

IshanChhangani commented Sep 4, 2024

Jira Link: DB-12692

Description

The checkAshData and testCircularBufferWrapAround tests fail in the ASAN build when a query bundle is triggered for a specific queryid, but no subsequent query with the same queryid is executed.

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.
@IshanChhangani IshanChhangani added area/ysql Yugabyte SQL (YSQL) status/awaiting-triage Issue awaiting triage labels Sep 4, 2024
@IshanChhangani IshanChhangani self-assigned this Sep 4, 2024
@yugabyte-ci yugabyte-ci added kind/bug This issue is a bug priority/medium Medium priority issue and removed status/awaiting-triage Issue awaiting triage labels Sep 4, 2024
IshanChhangani added a commit that referenced this issue Sep 25, 2024
…when no query executed

Summary:
- Fixes issue where pgss_copy.query_len being 0 leads to test failures, especially in ASAN builds, when a diagnostics bundle is triggered but no query is executed.
- Adds logic to handle cases where pg_stat_statements_reset() is called during the bundle's duration, dumping only counters data with an empty query string.
- Resolves problem of losing the last character in the pg_stat_statements.csv file.
- Modifies the behavior of GetAshRangeIndexes() to append the description "No data available in ASH for the given time range" instead of overwriting the existing catalog description.
- Removes unnecessary AddinShmemInitLock lock to improve efficiency in this scenario.
- Even in the case :-
```
bundle started.
query executed
pgss reset
query executed
bundle ended
```
`pg_stat_statements was reset, query string not available;` warning is displayed through yb_query_diagnostics_status. This is intentional as if we were to implement a check for last_time_query_bundled against last_time_reset, it would require a `GetCurrentTimestamp()` call per bundled query, which could be expensive.
Jira: DB-12692

Test Plan:
./yb_build.sh --java-test TestYbQueryDiagnostics#checkAshData
./yb_build.sh --java-test TestYbQueryDiagnostics#checkPgssData

Reviewers: asaha, telgersma

Reviewed By: asaha, telgersma

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D37895
timothy-e pushed a commit that referenced this issue Sep 26, 2024
Summary:
 35b12d2 [PLAT-15404] Average YSQL operations latency alert is using incorrect units (ms vs microsecs)
 Excluded: 008f885 [#23788] YSQL, QueryDiagnostics: Fixing issues in pg_stat_statements when no query executed
 6ca8cc4 [#23810] yugabyted-ui: UI is displaying incorrect disk size when multiple data directories
 dca5923 [PLAT-15034][K8s] Add changes to apply master_join_existing_cluster gflag
 fa9b370 [docs] Update content for getting started page for CDC logical replication (#23916)
 8db0ffb [PLAT-15380] clock drift alert did not reference nodes
 44ae377 [PLAT-15349] Mark universe update as success after update lb config
 Excluded: 9f90819 [#24121] xCluster: Fix xcluster_outbound_replication_group-itest TestGetStreamByTableId
 250a4d5 [#24026] docdb: Fix SIGSEGV from MaxPersistentOpId after flush
 0d1046a [DEVOPS-3238] Move macOS build to macos13 (Ventura)
 87cffc6 [#24137] DocDB: Add gflag_allowlist to yb_release_manifest
 678d277 [#21178] docdb: Add metric for the max master follower heartbeat delay.
 ff97f51 [doc][ybm] Certificate links (#24139)
 Excluded: d26b62d [#21733] YSQL: ParallelAppend and pg_hint_plan
 3ffe5a7 [PLAT-10519]Lack of Client-Side Inactivity Timeout - Part 1
 254e164 [PLAT-15432] remove status,sizeInBytes from manifest.json file

Test Plan: Jenkins: rebase: pg15-cherrypicks

Reviewers: tfoucher, fizaa, telgersma

Differential Revision: https://phorge.dev.yugabyte.com/D38454
IshanChhangani added a commit that referenced this issue Sep 27, 2024
…sues in pg_stat_statements when no query executed

Summary:
- pg_stat_statements.c
 - pg_stat_statements_reset()
  - YB master 008f885 updates `yb_pgss_last_reset_time` var.
  - YB PG15 55782d5 adds params to `entry_reset` function.
  - kept a combination of both by keeping the pg15 code and adding the update `yb_pgss_last_reset_time` statement at the start of the function.

original summary:-
- Fixes issue where pgss_copy.query_len being 0 leads to test failures, especially in ASAN builds, when a diagnostics bundle is triggered but no query is executed.
- Adds logic to handle cases where pg_stat_statements_reset() is called during the bundle's duration, dumping only counters data with an empty query string.
- Resolves problem of losing the last character in the pg_stat_statements.csv file.
- Modifies the behavior of GetAshRangeIndexes() to append the description "No data available in ASH for the given time range" instead of overwriting the existing catalog description.
- Removes unnecessary AddinShmemInitLock lock to improve efficiency in this scenario.
- Even in the case :-
```
bundle started.
query executed
pgss reset
query executed
bundle ended
```
`pg_stat_statements was reset, query string not available;` warning is displayed through yb_query_diagnostics_status. This is intentional as if we were to implement a check for last_time_query_bundled against last_time_reset, it would require a `GetCurrentTimestamp()` call per bundled query, which could be expensive.
Jira: DB-12692

Original commit: 008f885 / D37895

Test Plan:
./yb_build.sh --java-test TestYbQueryDiagnostics#checkAshData
./yb_build.sh --java-test TestYbQueryDiagnostics#checkPgssData

Reviewers: tfoucher, fizaa, telgersma

Reviewed By: fizaa

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D38458
IshanChhangani added a commit that referenced this issue Oct 22, 2024
…in pg15

Summary:
PostgreSQL 15 introduces a new function, pg_stat_statements_reset_1_7, to handle the pg_stat_statements_reset functionality. As a result, the current changes replicate the existing logic into this new function to maintain compatibility with the updated system.
Jira: DB-12692

Test Plan: ./yb_build.sh --java-test 'org.yb.pgsql.TestYbQueryDiagnostics#testPgssResetBetweenDiagnostics'

Reviewers: asaha

Reviewed By: asaha

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D39135
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