Skip to content
This repository has been archived by the owner on Nov 14, 2024. It is now read-only.

Fix media DB possibly leaking connections #3372

Merged
merged 1 commit into from
Jul 27, 2024
Merged

Fix media DB possibly leaking connections #3372

merged 1 commit into from
Jul 27, 2024

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented May 10, 2024

Grafana Pyroscope unveiled that we are hitting https://github.com/golang/go/blob/ad10fbd3c4ec7413775028213f4d5089b18926f7/src/database/sql/sql.go#L2739-L2742 for media DB queries.

Making the methods pointer receivers fixes this.

(Also some minor error cosmetic updates)

@S7evinK S7evinK added C-Media-API T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels May 10, 2024
@S7evinK S7evinK requested a review from a team as a code owner May 10, 2024 19:47
Copy link

codecov bot commented May 10, 2024

Codecov Report

Attention: Patch coverage is 66.66667% with 4 lines in your changes missing coverage. Please review.

Project coverage is 67.52%. Comparing base (46902e5) to head (6f5ab8f).
Report is 1 commits behind head on main.

Files Patch % Lines
mediaapi/storage/shared/mediaapi.go 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3372      +/-   ##
==========================================
+ Coverage   67.51%   67.52%   +0.01%     
==========================================
  Files         521      521              
  Lines       47366    47362       -4     
==========================================
+ Hits        31977    31982       +5     
+ Misses      11405    11394      -11     
- Partials     3984     3986       +2     
Flag Coverage Δ
unittests 52.76% <66.66%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@devonh devonh left a comment

Choose a reason for hiding this comment

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

Seems weird that it ever wasn't pointer receivers 🤷
Looks good!

@S7evinK S7evinK merged commit 795c4a9 into main Jul 27, 2024
19 of 20 checks passed
@S7evinK S7evinK deleted the s7evink/media-db branch July 27, 2024 20:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
C-Media-API T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants