Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Add index to optimize for list task executions for node execution #120

Merged
merged 1 commit into from
Sep 1, 2020
Merged

Add index to optimize for list task executions for node execution #120

merged 1 commit into from
Sep 1, 2020

Conversation

katrogan
Copy link
Contributor

@katrogan katrogan commented Sep 1, 2020 •
edited
Loading

TL;DR

Add index to optimize for list task executions for node execution

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Tracking Issue

flyteorg/flyte#328

Follow-up issue

NA

@codecov-commenter
Copy link

Codecov Report

Merging #120 into master will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #120      +/-   ##
==========================================
- Coverage   62.39%   62.34%   -0.05%     
==========================================
  Files         104      104              
  Lines        7752     7758       +6     
==========================================
  Hits         4837     4837              
- Misses       2345     2351       +6     
  Partials      570      570
Flag Coverage Δ
#unittests 62.34% <0.00%> (-0.05%) ⬇️

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

Impacted Files Coverage Δ
pkg/repositories/config/migrations.go 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bf159e5...11439d9. Read the comment docs.

@anandswaminathan
Copy link
Contributor

@katrogan The gorm joins are fine? Has the latency gone down completely?

@wild-endeavor
Copy link
Contributor

What happens when you try this locally? Do you see a performance bump?

Ideally, I feel like this is a gorm issue rather than a postgres issue, is there no way to skip those tables when not necessary? The query that gorm is delivering to postgres has the tables right?

@katrogan
Copy link
Contributor Author

katrogan commented Sep 1, 2020

@anandswaminathan yeah, i'm seeing latency on the order of formerly seconds to 0.1 ms now

@katrogan
Copy link
Contributor Author

katrogan commented Sep 1, 2020

@wild-endeavor I was misguided in the original reference bug - the query optimizer does indeed handle eliding the joins. adding this index improved query performance significantly

@katrogan katrogan merged commit ff64ab8 into master Sep 1, 2020
@wild-endeavor
Copy link
Contributor

+1

schottra added a commit that referenced this pull request Sep 8, 2020
@schottra
Merge branch 'master' into Lyft-Flyte
9cd682d 
* master:
  Gcs remote data (#121)
  Do not depend on GOPATH to locate test data (#122)
  Add index to optimize for list task executions for node execution (#120)
  Grpc health checking (#118)
  Allow random cluster selection when no override (#117)
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
@katrogan
Add index to optimize for list task executions for node execution (#120)
141259a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@anandswaminathan anandswaminathan anandswaminathan approved these changes

@kumare3 kumare3 Awaiting requested review from kumare3 kumare3 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@katrogan @codecov-commenter @anandswaminathan @wild-endeavor