-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
perf: Add template validation caching #13633
Conversation
fc3060e
to
a678d10
Compare
During template validation k8s API is called for each templateRef. For complex workflows with many refs it creates huge overhead. Let's cache such templates Throughout codebase each wrapper is used only in context of single call. It is not guaranteed for future, so TTL mechanism was added with default TTL=1M Additionally WithCacheTTL enables overwriting this parameter. Signed-off-by: Jakub Buczak <[email protected]>
a678d10
to
9df7abf
Compare
Benchmarking creation of workflows with multiple-ref templateSetupBranches:
Using fresh Argo server started with Before each tests following procedure were followed:
Benchmarking tool: hey. It runs command in parallel by default 200 times using 50 workers. Those values can be modified using:
Typical call: hey \
-n 200 -c 50 \
-m POST \
-disable-keepalive \
-T "application/json" \
-d '{
"serverDryRun": false,
"workflow": {
"metadata": {
"generateName": "curl-echo-test-",
"namespace": "argo-test"
},
"spec": {
"workflowTemplateRef": {"name": "20-echos"},
"arguments": {}
}
}
}' \
https://localhost:2746/api/v1/workflows/argo-test Results
AppendixWorkflowsWorkflowsapiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: echo-1
spec:
entrypoint: print-message
templates:
- name: print-message
inputs:
parameters:
- name: message
value: "hello world!"
container:
image: docker/whalesay
command: [cowsay]
args: ["{{inputs.parameters.message}}"] apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: echo-2
spec:
entrypoint: print-message
templates:
- name: print-message
inputs:
parameters:
- name: message
value: "hello world!"
container:
image: docker/whalesay
command: [cowsay]
args: ["{{inputs.parameters.message}}"] apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: 20-echos
spec:
entrypoint: start
activeDeadlineSeconds: 1800
templates:
- name: start
dag:
tasks:
- name: echo-1
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-1
- name: echo-2
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-2
- name: echo-3
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-3
- name: echo-4
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-4
- name: echo-5
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-5
- name: echo-6
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-6
- name: echo-7
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-7
- name: echo-8
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-8
- name: echo-9
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-9
- name: echo-10
templateRef:
name: echo-1
template: print-message
arguments:
parameters:
- name: message
value: echo-10
- name: echo-11
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-11
- name: echo-12
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-12
- name: echo-13
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-13
- name: echo-14
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-14
- name: echo-15
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-15
- name: echo-16
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-16
- name: echo-17
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-17
- name: echo-18
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-18
- name: echo-19
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-19
- name: echo-20
templateRef:
name: echo-2
template: print-message
arguments:
parameters:
- name: message
value: echo-20 `hey` output: Caching ON
|
wrapper.cacheMu.Lock() | ||
defer wrapper.cacheMu.Unlock() | ||
|
||
val, ok := wrapper.cache[name] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rather than a manual cache, this could probably accept a cache as an argument.
for instance, the Controller already has a cache of all WorkflowTemplates in its Informer.
the Server does not have an Informer yet, but will likely eventually have a SQLite cache similar to the one for the Workflow List
the CLI generally has none of the above though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes sense - I did this as a quick resolution.
Do you prefer to use the same pattern as with workflow list or just add template informer to the server?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#13672 is possibly fine for now, but the SQLite cache will eventually be needed for other features for filtering for WorkflowTemplates. It doesn't seem to be an often used part of the UI though, so maybe it's fine to leave without for now and modify when needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI: #13672 is ready for review!
I'm continuing the work in PR #13672 |
Motivation
Improve performance of creating workflows with complex templateRef structure.
During template validation k8s API is called for each templateRef. For complex workflows with many refs it creates huge overhead. Let's cache such templates
Connected to issue #7418
Modifications
Add caching for
WorkflowTemplates
andClusterWorkflowTemplates
with default TTL=1minVerification
I added test cases checking if caching is working + performed a benchmarks (results in a comment in this PR)