refactor: add indexes to secret informers to speedup settings parsing #7882
Conversation
38fa354 to
ffe32d8
Compare
Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
ffe32d8 to
a5a4d96
Compare
Codecov Report
@@ Coverage Diff @@
## master #7882 +/- ##
==========================================
- Coverage 41.48% 41.44% -0.04%
==========================================
Files 169 169
Lines 22254 22310 +56
==========================================
+ Hits 9232 9247 +15
- Misses 11703 11741 +38
- Partials 1319 1322 +3
Continue to review full report at Codecov.
|
…#7882) Signed-off-by: Alexander Matyushentsev <AMatyushentsev@gmail.com>
|
@yeya24 , yes - it addresses this issue as well as some other performance bottlenecks. I was fixing it in rush, since we also run into it in production and forgot to notify you. BTW I discovered some additional bottlenecks using your pprof endpoints - thank you! |
|
I am still seeing a lot of time spent on this function. Does ArgoCD still serialize secret after this pr has been merged? |
|
It might be that I missed something and we still have a method in controller that doesn't use the index to access the cluster. |
|
Hi @alexmt, that's the profile https://share.polarsignals.com/fbdddca/. You can switch to |
|
Thank you @yeya24 ! Yes, there is an issue I did not notice. It affects applications that use cluster name instead of URL. The fix is trivial - I need to introduce one more index (by cluster name). Working on fix. Will cherry-pick it into v2.2 |
Signed-off-by: Alexander Matyushentsev AMatyushentsev@gmail.com
PR improves settings parsing - introduces 3 indexes to secret informer: repo by project, cluster by project, cluster by server URL. The indexes allow the controller to avoid iterating an in-memory list of secrets which reduces CPU usage by 40%~50% on an instance which have ~400 secrets and 3k apps.
Additionally PR fixes a security issue: shared credentials are not supported but project repositories still "inherit" credentials configured in shared credentials. This is incorrect since the user can use project scoped repo to "steal" shared credentials.