argo-workflows
argo-workflows copied to clipboard
feat: Workflows list page performance improvements
Fixes #12025
Motivation
A performance issue occurred when merging the workflows and archived workflows pages into one place, and the logic was changed to resolve this.
Modifications
- When archived, an archived label is attached to the k8s resource, so the performance problem caused by the entire search was solved by changing the logic using the labelSelector of k8s.
- To manage the pagination values of workflows and archived workflows separately, a value called paginationOptions was added to both the front and back.
Verification
- created a workflows resource with archived enabled.
- checked that the data was not displayed in duplicate.
- checked if it operates normally when changing the number of pages and clicking the
next page
andfirst page
buttons. - checked whether all test codes pass without any problems
@sunyeongchoi are u able to include fix for https://github.com/argoproj/argo-workflows/issues/12161 too?
@sunyeongchoi are u able to include fix for #12161 too?
Yes. But I will take a look after this PR is completely finished.
I found a bug in the code I implemented. (Cronjob, event, and cli commands were also using a workflow list, but I realized that this had not been considered in this PR.)
As a result, the current PR is not complete and plan to add additional commit.
However, I would like to receive a review to see if the PR I implemented is in line with the direction of resolving the issue.
for example, when combining k8s workflows and archived workflows to show them on one page, the PaginationOptions
parameter was added to the parameters, and the names were set to WfContinue
for k8s workflows continue and ArchivedContinue
for archived workflows continue.
I would like to receive reviews on whether you think this name is right or if there are other good names.
When archived, an archived label is attached to the k8s resource, so the performance problem caused by the entire search was solved by changing the logic using the labelSelector of k8s.
Could you elaborate why this would solve the performance problem? If the number of archived workflows is large, it seems like we would still encounter the performance issue.
Could you elaborate why this would solve the performance problem? If the number of archived workflows is large, it seems like we would still encounter the performance issue.
Okay. A performance problem occurred because the total number of archived workflows
was searched.
So the logic I implemented is as follows:
- If the label "Archived" is attached to
k8s workflows
, it means that it is stored inarchived workflows
. Therefore, by adding the labelSelector syntax tok8s workflows
, workflows stored inarchived workflows
are not searched ink8s workflows
. (In other words, the possibility thatk8s workflows
andarchived workflows
may overlap was resolved through kubernetes labelSelector.)
wfLabelSelector += fmt.Sprintf("%s!=%s", common.LabelKeyWorkflowArchivingStatus, "Archived")
- If 10 pieces of data are needed on one page, if 6
k8s workflows
were searched in step 1, 10-6 = 4, that is, there is a possibility of searching 4 more data, so at this time, only search up to 4archived workflows
.
if int64(len(mergedWfs)) < limit {
listOption.Continue = archivedContinue
listOption.Limit = limit - int64(len(mergedWfs))
listOption.LabelSelector = labelSelector
archivedWfList, err = s.wfArchiveServer.ListArchivedWorkflows(ctx, &workflowarchivepkg.ListArchivedWorkflowsRequest{
ListOptions: listOption,
NamePrefix: "",
Namespace: req.Namespace,
})
}
- As described in number 1, deduplication was solved through Kubernetes' labelSelector, and the page limit was solved as described in number 2, so if
k8s workflows
andarchived workflows
manage only the continue value for each pagination separately, the performance problem will be solved becausek8s workflows
andarchived workflows
use their own pagination logic.
res := &wfv1.WorkflowList{ListMeta: metav1.ListMeta{ResourceVersion: wfList.ResourceVersion}, Items: mergedWfs, PaginationOptions: wfv1.WorkflowPaginationOptions{
WfContinue: common.CheckNilString(wfList.ListMeta.Continue),
ArchivedContinue: common.CheckNilString(archivedWfList.ListMeta.Continue),
}}
hi @sunyeongchoi , could you fix the DCO signature
hi @sunyeongchoi , could you fix the DCO signature
This still needs more work in any case, per above review.
Also per my last comment on the issue, Sun Yeong will be busy a few more days at least, please allow for some more time here. Per the same comment, we will have a front-end workaround out sooner
hi @sunyeongchoi , could you fix the DCO signature
This still needs more work in any case, per above review.
Also per my last comment on the issue, Sun Yeong will be busy a few more days at least, please allow for some more time here. Per the same comment, we will have a front-end workaround out sooner
Thanks for the update. I look forward to the front-end workaround
https://github.com/argoproj/argo-workflows/issues/12025#issuecomment-1949330952
I believe the concept of the issue has completely changed, so if everyone agrees, I will close this PR.
Thank you so much for your efforts so far!