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

feat: add sqlite-based memory store for live workflows. Fixes #12025 #12736

Merged
merged 42 commits into from
Apr 29, 2024
Merged

feat: add sqlite-based memory store for live workflows. Fixes #12025 #12736

merged 42 commits into from
Apr 29, 2024

Conversation

jiachengxu
Copy link
Member

@jiachengxu jiachengxu commented Mar 4, 2024

Fixes: #12025
Implementation of the proposal: #12025 (comment)

This PR implements a sqlite-based in-memory store that the workflow server will use to list live workflows, it provides:

  • more unified query availability, including ordering & filtering by date, filtering by labels, archived-workflow-like pagination. (Note that nameprefix is not added in this PR).
  • using List and Watch reflector so that workflow server won't tax Kubernetes per every user request.
  • new pagination method gets rid of the merging of live and archived workflows logic, and it doesn't require merging both live and archived workflows every time.

For testing the change of this PR, I have created a image and pushed to quay.io: https://quay.io/repository/jiachengxu/argocli, and you can replace the image of the argo-server container in the argo-server Deployment with quay.io/jiachengxu/argocli:fix-v6

Motivation

Modifications

Verification

@jiachengxu jiachengxu marked this pull request as ready for review March 7, 2024 16:41
Dockerfile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@terrytangyuan terrytangyuan enabled auto-merge (squash) April 29, 2024 01:32
@terrytangyuan terrytangyuan merged commit f1ab5aa into argoproj:main Apr 29, 2024
27 checks passed
@agilgur5 agilgur5 added area/api Argo Server API area/server labels Apr 29, 2024
Joibel added a commit to Joibel/argo-workflows that referenced this pull request May 7, 2024
@Joibel
Copy link
Member

Joibel commented May 8, 2024

@jiachengxu unfortunately I've reverted this commit in #13018
The CI was not correctly testing the argocli image as part of the PR, and ListWorkflows in some of the tests using argocli is returning an empty list (@isubasinghe has demonstrated this).
It'll need fixing up and resubmitting on top of #13018 which should test the CLI properly from now on.

Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

Follow-up to Alan's comments above and my own root cause analysis that led up to the revert, I suspect a very tiny piece of this PR was causing the CLI bug

pkg/apiclient/argo-kube-client.go Show resolved Hide resolved
pkg/apiclient/argo-kube-client.go Show resolved Hide resolved
@agilgur5 agilgur5 added this to the v3.5.x patches milestone May 8, 2024
agilgur5 pushed a commit that referenced this pull request May 27, 2024
…12736)

Signed-off-by: Jiacheng Xu <[email protected]>
Co-authored-by: Anton Gilgur <[email protected]>
(cherry picked from commit f1ab5aa)
@agilgur5
Copy link
Contributor

agilgur5 commented May 27, 2024

Backported into release-3.5 as 50dc580

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api Argo Server API area/server prioritized-review For members of the Sustainability Effort
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.5 ListWorkflows causes server to hang when there are lots of archived workflows
8 participants