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

Fix sys.dm_exec_sessions having rows for backends which are already terminated #3329

Merged

Conversation

tanscorpio7
Copy link
Contributor

@tanscorpio7 tanscorpio7 commented Dec 30, 2024

Description

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Issues Resolved

[BABEL-5414]

Sign Off

Signed-off-by: Tanzeel Khan [email protected]

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tanzeel Khan <[email protected]>
Signed-off-by: Tanzeel Khan <[email protected]>
@coveralls
Copy link
Collaborator

coveralls commented Dec 30, 2024

Pull Request Test Coverage Report for Build 12548175612

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.001%) to 74.865%

Files with Coverage Reduction New Missed Lines %
contrib/babelfishpg_tds/src/backend/tds/tds.c 2 92.66%
contrib/babelfishpg_tsql/src/session.c 5 97.44%
Totals Coverage Status
Change from base Build 12543004223: 0.001%
Covered Lines: 46599
Relevant Lines: 62244

💛 - Coveralls

Signed-off-by: Tanzeel Khan <[email protected]>
@tanscorpio7 tanscorpio7 changed the title try fix for sys.dm_exec_sessions Fix sys.dm_exec_sessions having garbage entries Dec 30, 2024
@tanscorpio7 tanscorpio7 changed the title Fix sys.dm_exec_sessions having garbage entries Fix sys.dm_exec_sessions having rows for backends which are already terminated Dec 30, 2024
Copy link
Contributor

@shah-nirmit shah-nirmit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shardgupta shardgupta merged commit df1ce4d into babelfish-for-postgresql:BABEL_5_X_DEV Dec 30, 2024
45 checks passed
tanscorpio7 added a commit to tanscorpio7/babelfish_extensions that referenced this pull request Dec 30, 2024
…erminated (babelfish-for-postgresql#3329)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
tanscorpio7 added a commit to tanscorpio7/babelfish_extensions that referenced this pull request Dec 30, 2024
…erminated (babelfish-for-postgresql#3329)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
tanscorpio7 added a commit to tanscorpio7/babelfish_extensions that referenced this pull request Dec 30, 2024
…erminated (babelfish-for-postgresql#3329)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
shardgupta pushed a commit that referenced this pull request Dec 31, 2024
…erminated (#3329) (#3333)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
shardgupta pushed a commit that referenced this pull request Dec 31, 2024
…erminated (#3329) (#3332)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
shardgupta pushed a commit that referenced this pull request Dec 31, 2024
…erminated (#3329) (#3331)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
pranavJ23 pushed a commit to pranavJ23/babelfish_extensions that referenced this pull request Jan 6, 2025
…erminated (babelfish-for-postgresql#3329)

We initialize and keep a local status array which has the entries
for all TDS connections but we did not reset these entries during
connection abort or shmem exit of the backend, which means
future read of this local status array will see these entries as valid.

As a fix, mark the status entry as invalid during proc/shmem exit

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan [email protected]
shardgupta pushed a commit that referenced this pull request Jan 6, 2025
…dm_exec_sessions (#3350)

#3329 was not enough to fix abandoned rows in sys.dm_exec_sessions. Sessions terminated using kill command still leave behind an entry in sys.dm_exec_sessions.

Root cause was not executing the shmem exit hook when session terminated because of SIGTERM.

As a fix make `tds_stats_shmem_shutdown` completely similar to `pgstat_beshutdown_hook`.

Task: [BABEL-5414]

Signed-off-by: Tanzeel Khan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants