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: application list api is very slow when fields query parameter is supplied #11250

Merged
merged 3 commits into from Nov 16, 2022

Conversation

alexmt
Copy link
Collaborator

@alexmt alexmt commented Nov 10, 2022

Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com

Depends on argoproj/pkg#243

Application list API method uses gRPC -> HTTP forwarder that provides custom fields functionality. It appears that fields processing is pretty slow. For example API response that returns 5k apps takes ~2 seconds and ~1.8 seconds is required to remove not selected fields.

Implemented PR uses "custom field processor": non generic function that only support specific set of fields. The function is not generic but significantly improves performance of applications list API ( ~ 10x )

… supplied

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 45.64% // Head: 45.64% // No change to project coverage 👍

Coverage data is based on head (315a1e2) compared to base (41b1ebe).
Patch has no changes to coverable lines.

❗ Current head 315a1e2 differs from pull request most recent head de518ca. Consider uploading reports for the commit de518ca to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11250   +/-   ##
=======================================
  Coverage   45.64%   45.64%           
=======================================
  Files         239      239           
  Lines       28991    28991           
=======================================
  Hits        13233    13233           
  Misses      13939    13939           
  Partials     1819     1819           
Impacted Files Coverage Δ
util/settings/settings.go 51.25% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@@ -17,6 +18,83 @@ import (
"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
)

var appFields = map[string]func(app *v1alpha1.Application) interface{}{
Copy link
Member

@jessesuen jessesuen Nov 15, 2022

Choose a reason for hiding this comment

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

Since this is a var that needs to be periodically maintained, for future reference, it would be good to explain what this is for, how its used by UI, and how the fields were chosen and when they need to be updated.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point. Added comment here and in UI code as well

…st/watch api

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
@alexmt alexmt merged commit 9bd0ed0 into argoproj:master Nov 16, 2022
@alexmt alexmt deleted the app-list-performance branch November 16, 2022 17:42
alexmt added a commit to alexmt/argo-cd that referenced this pull request Nov 17, 2022
… supplied (argoproj#11250)

* fix: application list api is very slow when fields query parameter is supplied

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* regenerate docs

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* document the requirement to maintain support fields of application list/watch api

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
ashutosh16 pushed a commit to ashutosh16/argo-cd that referenced this pull request Nov 23, 2022
… supplied (argoproj#11250)

* fix: application list api is very slow when fields query parameter is supplied

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* regenerate docs

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* document the requirement to maintain support fields of application list/watch api

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
emirot pushed a commit to emirot/argo-cd that referenced this pull request Jan 27, 2023
… supplied (argoproj#11250)

* fix: application list api is very slow when fields query parameter is supplied

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* regenerate docs

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

* document the requirement to maintain support fields of application list/watch api

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>

Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
Signed-off-by: emirot <emirot.nolan@gmail.com>
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.

None yet

2 participants