Skip to content

Reset unusable db connections#6725

Merged
shwstppr merged 1 commit intoapache:4.16from
mrog:reset_db_connections
Sep 13, 2022
Merged

Reset unusable db connections#6725
shwstppr merged 1 commit intoapache:4.16from
mrog:reset_db_connections

Conversation

@mrog
Copy link
Copy Markdown
Contributor

@mrog mrog commented Sep 9, 2022

Description

This PR...

When the management service detects a broken database connection, it didn't do anything about it. It already has code to reset the connection, but that doesn't get used. This change causes the connection to be reset after it's found to be unusable.

See #6724 for repro steps

Fixes: 6724

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

The instructions in INSTALL.md are incomplete or outdated. I was able to get the project to build, but I haven't been able to test my change. See #6726.

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Sep 11, 2022

Thanks @mrog for the PR, sounds like good find, let us kick some regression tests.
@blueorangutan package

@blueorangutan
Copy link
Copy Markdown

@rohityadavcloud a Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link
Copy Markdown

Packaging result: ✔️ el7 ✔️ el8 ✔️ debian ✔️ suse15. SL-JID 4162

@yadvr
Copy link
Copy Markdown
Member

yadvr commented Sep 11, 2022

@blueorangutan test matrix

@blueorangutan
Copy link
Copy Markdown

@rohityadavcloud a Trillian-Jenkins matrix job (centos7 mgmt + xs71, centos7 mgmt + vmware65, centos7 mgmt + kvmcentos7) has been kicked to run smoke tests

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-4863)
Environment: xenserver-71 (x2), Advanced Networking with Mgmt server 7
Total time taken: 29891 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6725-t4863-xenserver-71.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-4865)
Environment: vmware-65u2 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33424 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6725-t4865-vmware-65u2.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

@blueorangutan
Copy link
Copy Markdown

Trillian test result (tid-4864)
Environment: kvm-centos7 (x2), Advanced Networking with Mgmt server 7
Total time taken: 33362 seconds
Marvin logs: https://github.com/blueorangutan/acs-prs/releases/download/trillian/pr6725-t4864-kvm-centos7.zip
Smoke tests completed. 92 look OK, 0 have errors
Only failed tests results shown below:

Test Result Time (s) Test File

Copy link
Copy Markdown
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

@DaanHoogland DaanHoogland added this to the 4.17.1.0 milestone Sep 12, 2022
@shwstppr shwstppr closed this Sep 13, 2022
@shwstppr shwstppr reopened this Sep 13, 2022
@shwstppr
Copy link
Copy Markdown
Contributor

Previous Travis failures seem unrelated. Will check with 4.17.1 health check. Merging this.

@shwstppr shwstppr merged commit 77df050 into apache:4.16 Sep 13, 2022
@boring-cyborg
Copy link
Copy Markdown

boring-cyborg bot commented Sep 13, 2022

Awesome work, congrats on your first merged pull request!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants