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

jiachengxu · 2024-03-04T15:50:23Z

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

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

server/workflow/store/sqlite_store.go

Dockerfile

Makefile

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

jessesuen

LGTM, but wondering if we have an off-by-one bug?

server/workflow/store/sqlite_store.go

persist/sqldb/selector.go

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

jessesuen

Great work!

terrytangyuan

Looks great! Thanks for the hardwork @jiachengxu!

sarabala1979

LGTM

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

…Fixes argoproj#12025 (argoproj#12736)" This reverts commit f1ab5aa. Signed-off-by: Alan Clucas <alan@clucas.org>

Joibel · 2024-05-08T08:14:00Z

@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.

agilgur5

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

jiachengxu added 2 commits March 4, 2024 23:42

feat: add sqlite-based memory store for live workflows

8793b7a

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

fix: lint

55cd632

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

agilgur5 mentioned this pull request Mar 5, 2024

UI: Allow sorting workflows by more criteria #3157

Open

terrytangyuan requested a review from jessesuen March 5, 2024 04:52

jiachengxu added 3 commits March 7, 2024 21:04

chore(test): update the test

3f76d4f

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore(test): try to set cgo enabled for go sqlite

bdb010d

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore: remove direct use of go-sqlite3

a451490

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

jiachengxu force-pushed the list-workflow branch from 7337c79 to a451490 Compare March 7, 2024 14:58

jiachengxu added 2 commits March 7, 2024 23:28

chore: add build-base to use cgo in builder image

e6d3d25

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore: small fix for test

3eac5a4

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

jiachengxu marked this pull request as ready for review March 7, 2024 16:41

jiachengxu commented Mar 7, 2024

View reviewed changes

server/workflow/store/sqlite_store.go Outdated Show resolved Hide resolved

jiachengxu mentioned this pull request Mar 7, 2024

3.5 ListWorkflows causes server to hang when there are lots of archived workflows #12025

Closed

3 tasks

ryancurrah reviewed Mar 7, 2024

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

jiachengxu added 13 commits March 11, 2024 21:59

chore: get rid of cgo

abc06d3

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore: revert the makefile change

ae50e97

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore: revert the makefile change

05e4029

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore: update

c944a61

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

fix: lint

f322316

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore(stage): add test

f00720d

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

Merge branch 'main' into list-workflow

2f0e02e

chore(stage): fix lint

9647006

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

feat: implement pagination

85665e3

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

fix: unit test

e0df0ff

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

fix: lint

bafa4f1

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

Merge branch 'main' into list-workflow

9883a3c

fix: e2e test

b2b4fe2

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

This was referenced Mar 25, 2024

Workflow list pagination by date completed #2926

Closed

feat(ui): set pagination limit to 100 by default #3565

Closed

Poor UI performance with lots of workflows viewing all. Random sort order when viewing paginated. #4546

Closed

jiachengxu added 3 commits April 11, 2024 17:43

Merge remote-tracking branch 'upstream/main' into list-workflow

fc431ed

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

fix: import

67ae8f9

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

Merge branch 'main' into list-workflow

c8b850c

jiachengxu changed the title ~~feat: add sqlite-based memory store for live workflows~~ feat: add sqlite-based memory store for live workflows. Fixes #12025 Apr 12, 2024

jiachengxu added 4 commits April 12, 2024 22:16

Merge branch 'main' into list-workflow

df6fdf5

Merge remote-tracking branch 'upstream/main' into list-workflow

68b33f4

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

fix: import

9c8149b

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

chore: empty commit

604d689

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

jessesuen reviewed Apr 22, 2024

View reviewed changes

server/workflow/store/sqlite_store.go Show resolved Hide resolved

persist/sqldb/selector.go Outdated Show resolved Hide resolved

persist/sqldb/selector.go Outdated Show resolved Hide resolved

jiachengxu added 2 commits April 24, 2024 18:30

fix: address PR comment

d1f0ace

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

Merge remote-tracking branch 'upstream/main' into list-workflow

1001279

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

jessesuen approved these changes Apr 25, 2024

View reviewed changes

terrytangyuan approved these changes Apr 26, 2024

View reviewed changes

sarabala1979 approved these changes Apr 28, 2024

View reviewed changes

terrytangyuan enabled auto-merge (squash) April 28, 2024 06:54

Merge remote-tracking branch 'upstream/main' into list-workflow

66bcc6e

Signed-off-by: Jiacheng Xu <xjcmaxwellcjx@gmail.com>

auto-merge was automatically disabled April 29, 2024 01:29
Head branch was pushed to by a user without write access

terrytangyuan enabled auto-merge (squash) April 29, 2024 01:32

terrytangyuan merged commit f1ab5aa into argoproj:main Apr 29, 2024
27 checks passed

terrytangyuan mentioned this pull request Apr 29, 2024

Release v3.5 patch releases discussion #11997

Open

agilgur5 mentioned this pull request Apr 29, 2024

3.5 Pagination may not work correctly for archived workflows #11715

Closed

3 tasks

agilgur5 added area/api Argo Server API area/server labels Apr 29, 2024

agilgur5 mentioned this pull request May 4, 2024

[testing PR] test why e2e tests are failing #13002

Closed

Joibel added a commit to Joibel/argo-workflows that referenced this pull request May 7, 2024

chore: Revert "feat: add sqlite-based memory store for live workflows. …

584d51e

…Fixes argoproj#12025 (argoproj#12736)" This reverts commit f1ab5aa. Signed-off-by: Alan Clucas <alan@clucas.org>

Joibel mentioned this pull request May 7, 2024

fix: use argocli image from PR in CI + revert #12736 #13018

Merged

agilgur5 reviewed May 8, 2024

View reviewed changes

pkg/apiclient/argo-kube-client.go Show resolved Hide resolved

pkg/apiclient/argo-kube-client.go Show resolved Hide resolved

jiachengxu mentioned this pull request May 8, 2024

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

Merged

agilgur5 added this to the v3.5.x patches milestone May 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

jiachengxu commented Mar 4, 2024 •

edited

jessesuen left a comment

jessesuen left a comment

terrytangyuan left a comment

sarabala1979 left a comment

Joibel commented May 8, 2024

agilgur5 left a comment

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

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

Conversation

jiachengxu commented Mar 4, 2024 • edited

Motivation

Modifications

Verification

jessesuen left a comment

Choose a reason for hiding this comment

jessesuen left a comment

Choose a reason for hiding this comment

terrytangyuan left a comment

Choose a reason for hiding this comment

sarabala1979 left a comment

Choose a reason for hiding this comment

Joibel commented May 8, 2024

agilgur5 left a comment

Choose a reason for hiding this comment

jiachengxu commented Mar 4, 2024 •

edited