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: warehouse router tracker #5407

Merged
merged 2 commits into from
Jan 8, 2025
Merged

Conversation

achettyiitr
Copy link
Member

@achettyiitr achettyiitr commented Jan 5, 2025

Description

  • In the previous implementation, we are sending the warehouse_track_upload_missing Guage metrics twice. First with value 0 and then if missing upload exists we send value with 1.
  • The way alerts works is, if the value 1 is continuously fired for 2H then the alert is being sent. But if the value comes as 0 it will get resolved. For calculating the value it uses whatever it being present at that instant.

Linear Ticket

  • Resolves WAR-114

Security

  • The code changed/added as part of this pull request won't create any security issues with how the software is being used.

@achettyiitr achettyiitr force-pushed the fix.router-tracker branch 2 times, most recently from 4b17736 to 1f6a706 Compare January 5, 2025 21:44
Copy link

codecov bot commented Jan 5, 2025

Codecov Report

Attention: Patch coverage is 93.49593% with 8 lines in your changes missing coverage. Please review.

Project coverage is 74.73%. Comparing base (29b279d) to head (b436b44).
Report is 2 commits behind head on release/1.40.x.

Files with missing lines Patch % Lines
warehouse/router/tracker.go 93.00% 6 Missing and 1 partial ⚠️
warehouse/router/identities.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##           release/1.40.x    #5407      +/-   ##
==================================================
- Coverage           74.80%   74.73%   -0.07%     
==================================================
  Files                 440      440              
  Lines               61442    61412      -30     
==================================================
- Hits                45963    45899      -64     
- Misses              12950    12972      +22     
- Partials             2529     2541      +12     

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

warehouse/router/tracker.go Show resolved Hide resolved
warehouse/router/router.go Show resolved Hide resolved
warehouse/router/tracker.go Show resolved Hide resolved
Comment on lines +141 to +147
SELECT EXISTS (
SELECT 1
FROM ` + whutils.WarehouseUploadsTable + `
WHERE source_id = $1 AND destination_id = $2 AND
(status = $3 OR status = $4 OR status LIKE $5) AND
updated_at > $6
);`

Choose a reason for hiding this comment

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

Was just checking indexes for uploads table.

  1. Do we keep deleting old entries from UploadsTable else without proper index on time columns this query can keep getting slower with time.
  2. Not relevant to this PR. CREATE INDEX wh_uploads_status_index ON wh_uploads(status text_ops); index on status column with very low cardinality could have more performance drawbacks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we keep deleting old entries from UploadsTable else without proper index on time columns this query can keep getting slower with time.

Yes.

func (a *Archiver) Delete(ctx context.Context) error {

Copy link
Member Author

Choose a reason for hiding this comment

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

Not relevant to this PR. CREATE INDEX wh_uploads_status_index ON wh_uploads(status text_ops); index on status column with very low cardinality could have more performance drawbacks.

True, For columns with low cardinality, PostgreSQL might opt for a sequential scan over using the index, as scanning the table can be more efficient than traversing an index with low selectivity.

Copy link
Member Author

@achettyiitr achettyiitr Jan 8, 2025

Choose a reason for hiding this comment

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

Tracking Query-related optimizations/discussions in this doc.

@achettyiitr achettyiitr changed the base branch from master to release/1.40.x January 8, 2025 10:50
@achettyiitr achettyiitr merged commit 8e314b6 into release/1.40.x Jan 8, 2025
56 checks passed
@achettyiitr achettyiitr deleted the fix.router-tracker branch January 8, 2025 18:03
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.

3 participants