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

GetExecution performance improvements #171

Merged
merged 1 commit into from
Mar 30, 2021
Merged

GetExecution performance improvements #171

merged 1 commit into from
Mar 30, 2021

Conversation

katrogan
Copy link
Contributor

@katrogan katrogan commented Mar 30, 2021

Signed-off-by: Katrina Rogan katroganGH@gmail.com

TL;DR

See associated issue for more background: flyteorg/flyte#879

This change removes all ordering in point get queries by replacing gorm First with Take.

Also, removes the GetById call for relaunched workflow executions which was reconstructing the parent workflow execution identifier every time we called get on the relaunched exec. Instead we just serialize that value once when we actually process the request to relaunch the 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

see above

Tracking Issue

flyteorg/flyte#879

Follow-up issue

NA

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
@kumare3
Copy link
Contributor

kumare3 commented Mar 30, 2021

There are a lot of places where we use First, should we be using Take in all of these places

pkg/repositories/gormimpl/execution_repo.go:    }).First(&execution)
pkg/repositories/gormimpl/execution_repo.go:    }).First(&execution)
pkg/repositories/gormimpl/launch_plan_repo.go:  }).First(&launchPlan)
pkg/repositories/gormimpl/named_entity_repo.go: tx = tx.Select(getSelectForNamedEntity(tableName, input.ResourceType)).First(&namedEntity)
pkg/repositories/gormimpl/node_execution_repo.go:       }).Preload("ChildNodeExecutions").First(&nodeExecution)
pkg/repositories/gormimpl/project_repo.go:      }).First(&project)
pkg/repositories/gormimpl/resource_repo.go:     tx.Order(priorityDescending).First(&resources)
pkg/repositories/gormimpl/resource_repo.go:     }).First(&model)
pkg/repositories/gormimpl/task_execution_repo.go:       }).Preload("ChildNodeExecution").First(&taskExecution)
pkg/repositories/gormimpl/task_repo.go: }).First(&task)
pkg/repositories/gormimpl/workflow_repo.go:     }).First(&workflow)

@codecov
Copy link

codecov bot commented Mar 30, 2021

Codecov Report

Merging #171 (cf72854) into master (1ef63b1) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #171      +/-   ##
==========================================
+ Coverage   62.78%   62.82%   +0.03%     
==========================================
  Files         100      100              
  Lines        7288     7257      -31     
==========================================
- Hits         4576     4559      -17     
+ Misses       2126     2116      -10     
+ Partials      586      582       -4     
Flag Coverage Δ
unittests 62.82% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
pkg/repositories/transformers/execution.go 76.92% <ø> (+0.11%) ⬆️
pkg/manager/impl/execution_manager.go 68.87% <100.00%> (+0.57%) ⬆️
pkg/repositories/gormimpl/execution_repo.go 64.47% <100.00%> (-0.70%) ⬇️
pkg/repositories/gormimpl/launch_plan_repo.go 63.63% <100.00%> (ø)
pkg/repositories/gormimpl/named_entity_repo.go 69.47% <100.00%> (ø)
pkg/repositories/gormimpl/node_execution_repo.go 60.82% <100.00%> (ø)
pkg/repositories/gormimpl/project_repo.go 73.91% <100.00%> (ø)
pkg/repositories/gormimpl/task_execution_repo.go 60.49% <100.00%> (ø)
pkg/repositories/gormimpl/task_repo.go 68.11% <100.00%> (ø)
pkg/repositories/gormimpl/workflow_repo.go 69.11% <100.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 1ef63b1...cf72854. Read the comment docs.

@wild-endeavor
Copy link
Contributor

modulo ketan's comment.

@katrogan
Copy link
Contributor Author

PR already removes Take where appropriate

@katrogan katrogan merged commit 2db25e0 into master Mar 30, 2021
EngHabu pushed a commit that referenced this pull request Apr 26, 2021
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
EngHabu pushed a commit that referenced this pull request Apr 26, 2021
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>
EngHabu added a commit that referenced this pull request Apr 30, 2021
…D and OAuth2 configs, OAuth2 Metadata over gRPC #minor (#168)

* wip: OAuth2 Support

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* tighten security of generated tokens

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support storing form post values in auth code JWT

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* save secrets to k8s secrets

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Expose metadata endpoints over gRPC

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* trim OpenID Connect config further

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Selectively authenticate gRPC endpoints

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support external oauth2 server and Okta Config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* update config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix nil secrets data map

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fixed the pointer overwrite issue in oauthServer metadata (#183)

Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>

Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Simplify config further and move auth package up

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix clusterresource Project and domain(#167)

* Fix clusterresource Project

Signed-off-by: Anand Swaminathan <aswaminathan@lyft.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Bump flyteidl version to pick up auth role field number fix (#169)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add option to use project name as namespace for the task pods (#166)

* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* GetExecution performance improvements (#171)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add exists check for workflow & node executions (#172)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Remove legacy fetch for workflow execution inputs (#173)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added release workflow (#170)

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update Flyteidl version (#175)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added version in flyteadmin (#154)

* wip: added version pkg

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: resolve conflict

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added version in rpc

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: small fixes

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: Added panic cache in get version service

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added flytestdlib for version package

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added version service test

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added ldflags in goreleaser

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Propagate nesting and principal for child executions (#177)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Write workflow and node execution events asynchronously (#174)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add sensible flyteadmin config defaults (#179)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Lint

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* further cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Only register authserver when auth is enabled

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to latest flyteidl and separate auth interfaces

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* dead code

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* PR Comments

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* merge master

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Move to authorizedUris

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to released flyteidl

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix response expiry and add unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update go mod

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* fix unit tests that broke because of identity changes

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Co-authored-by: pmahindrakar-oss <77798312+pmahindrakar-oss@users.noreply.github.com>
Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Co-authored-by: Anand Swaminathan <aswaminathan@lyft.com>
Co-authored-by: Katrina Rogan <katrina@nuclyde.io>
Co-authored-by: Jeev B <jeevb@users.noreply.github.com>
Co-authored-by: Yuvraj <10830562+evalsocket@users.noreply.github.com>
Co-authored-by: Flyte Bot <admin@flyte.org>
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
…D and OAuth2 configs, OAuth2 Metadata over gRPC #minor (#168)

* wip: OAuth2 Support

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* wip

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* tighten security of generated tokens

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support storing form post values in auth code JWT

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* save secrets to k8s secrets

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Expose metadata endpoints over gRPC

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* trim OpenID Connect config further

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Selectively authenticate gRPC endpoints

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Support external oauth2 server and Okta Config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* update config

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix nil secrets data map

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fixed the pointer overwrite issue in oauthServer metadata (#183)

Signed-off-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>

Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Simplify config further and move auth package up

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix clusterresource Project and domain(#167)

* Fix clusterresource Project

Signed-off-by: Anand Swaminathan <aswaminathan@lyft.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Bump flyteidl version to pick up auth role field number fix (#169)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add option to use project name as namespace for the task pods (#166)

* Add option to use project name as namespace for the task pods

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>

* rename

Signed-off-by: Jeev B <jeev.balakrishnan@freenome.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* GetExecution performance improvements (#171)

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add exists check for workflow & node executions (#172)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Remove legacy fetch for workflow execution inputs (#173)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added release workflow (#170)

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update Flyteidl version (#175)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Added version in flyteadmin (#154)

* wip: added version pkg

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: resolve conflict

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added version in rpc

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: small fixes

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: Added panic cache in get version service

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added flytestdlib for version package

Signed-off-by: yuvraj <evalsocket@gmail.com>

* Added version service test

Signed-off-by: yuvraj <evalsocket@gmail.com>

* wip: added ldflags in goreleaser

Signed-off-by: yuvraj <evalsocket@gmail.com>
Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Propagate nesting and principal for child executions (#177)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Write workflow and node execution events asynchronously (#174)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Add sensible flyteadmin config defaults (#179)

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Lint

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* further cleanup

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Only register authserver when auth is enabled

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to latest flyteidl and separate auth interfaces

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* dead code

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* PR Comments

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* merge master

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Move to authorizedUris

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update to released flyteidl

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Fix response expiry and add unit tests

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* Update go mod

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

* fix unit tests that broke because of identity changes

Signed-off-by: Haytham Abuelfutuh <haytham@afutuh.com>

Co-authored-by: pmahindrakar-oss <77798312+pmahindrakar-oss@users.noreply.github.com>
Co-authored-by: Prafulla Mahindrakar <prafulla.mahindrakar@gmail.com>
Co-authored-by: Anand Swaminathan <aswaminathan@lyft.com>
Co-authored-by: Katrina Rogan <katrina@nuclyde.io>
Co-authored-by: Jeev B <jeevb@users.noreply.github.com>
Co-authored-by: Yuvraj <10830562+evalsocket@users.noreply.github.com>
Co-authored-by: Flyte Bot <admin@flyte.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants