Add support for display archived workflow on web ui from s3 archival store #5117

Aoao54 · 2023-11-15T11:52:05Z

What changed?
Add index workflowRunId to s3 archival store and support query on it.
Alter DescribeWorkflowExecution func to support describeArchivedWorkflowExecution.

Why?
Support for display archived workflow on web ui. Up to now, for the archived workflow which has past the retention can't displayed on the web ui.

How did you test it?
Add an unit test TestDescribeWorkflowExecution_ArchivedStatus .
And test the feature locally by opening archived wf which has past the retention on the web ui.

Potential risks

Is hotfix candidate?

CLAassistant · 2023-11-15T11:52:13Z

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

Aoao54 · 2023-11-16T01:51:33Z

Got something wrong with unit test,I'll find some time to fix it.

service/frontend/errors.go

service/frontend/workflow_handler.go

…store

yycptt

Thanks for the contribution!

I do have some concerns and questions in terms of the high level approach though. Happy to chat more about it.

yycptt · 2023-11-20T02:23:42Z

service/frontend/workflow_handler.go

+	for _, batch := range resp.HistoryBatches {
+		history.Events = append(history.Events, batch.Events...)
+	}
+	// reverse the events


I am a bit confused here. Looks like this is only reversing events within a page. When there are multiple pages, the returned history will look like something like:
event 100, 99, 98, ..., 1, event 200, 199, 198, ..., 101.

Yep.There will be a mistake. Mark it on my todo list.

I am not sure how you plan to resolve this though. It seems like a n^2 alg. if the underlying storage doesn't support reading backward.

Not sure whether if the underlying storage support it. I'll check it out. If not, maybe there's no smarter way.
I think I'll focus on this and learning the internals of mutable state for now. Since the other changes require more design discussion。：）

yycptt · 2023-11-20T02:29:16Z

service/frontend/workflow_handler.go

 		return true
 	}

 	return false
 }

+func (wh *WorkflowHandler) describeArchivedWorkflowExecution(ctx context.Context, request *workflowservice.DescribeWorkflowExecutionRequest) (*workflowservice.DescribeWorkflowExecutionResponse, error) {


I don't think archived workflow can be described based on visibility records. Fields like pending activities or pending children can't be populated by vis records.

If we really want to do it, the right way should be rebuild workflow mutable state from history events, then describe the rebuilt mutable states so that all fields in the describe response can be populated.

Alternatively, a separate API can be created for describing archived workflow and defined a new response that returns only limited information.

cc @yiminc

I don't think archived workflow can be described based on visibility records. Fields like pending activities or pending children can't be populated by vis records.

It seems a little weird if a archived workflow still has these pending things. Should we consider a archived wf is totally completed(without any changeable things)?

rebuild workflow mutable state from history events, then describe the rebuilt mutable states so that all fields in the describe response can be populated.

I'm not familiar with the internals of mutable state.. According to the previous experience issue, an archived workflow which has past the retention is considered deleted from mutable states. If we want to describe it based on mutable state, there maybe a lot of change to do.

A separate API is good. We need more discuss about the limited information of an archived wf and how to get it..

workflow can still have pending things even after closed, for example when workflow is force terminated before activity finishes. Archived or not is a concept of where the data is stored, so they are orthogonal.

there maybe a lot of change to do.
Yes it's a lot of work and change to rehydrate archived workflow back into the system. But, to me, that seems the right thing to do, and not causing more confusion to the caller of DescribeWorkflowExecution.

yycptt

@yiminc Do we have any established process for external contribution, especially for changes that require some design discussion? Maybe a PR to https://github.com/temporalio/proposals is the first step?

yycptt · 2023-11-21T01:03:02Z

service/frontend/workflow_handler.go

 		return true
 	}

 	return false
 }

+func (wh *WorkflowHandler) describeArchivedWorkflowExecution(ctx context.Context, request *workflowservice.DescribeWorkflowExecutionRequest) (*workflowservice.DescribeWorkflowExecutionResponse, error) {


workflow can still have pending things even after closed, for example when workflow is force terminated before activity finishes. Archived or not is a concept of where the data is stored, so they are orthogonal.

there maybe a lot of change to do.
Yes it's a lot of work and change to rehydrate archived workflow back into the system. But, to me, that seems the right thing to do, and not causing more confusion to the caller of DescribeWorkflowExecution.

yycptt · 2023-11-21T01:04:22Z

service/frontend/workflow_handler.go

+	for _, batch := range resp.HistoryBatches {
+		history.Events = append(history.Events, batch.Events...)
+	}
+	// reverse the events


I am not sure how you plan to resolve this though. It seems like a n^2 alg. if the underlying storage doesn't support reading backward.

jlegrone · 2023-12-18T17:47:37Z

Related issue on UI repo: temporalio/ui#1174

jontro · 2024-03-10T15:29:23Z

Why is changes required to temporal? The cli can show archived workflow executions and the web-ui supported this in the past.

Aoao54 · 2024-03-11T01:51:53Z

Hi @jontro , you can check this issues for more context.

In a word , the archived WF which has past the retention can't displayed on the web ui. Because the web ui read archived WF info from history store but it will be deleted after the retention. So there is a 404 error.

jontro · 2024-03-11T22:11:09Z

Hi @jontro , you can check this issues for more context.

In a word , the archived WF which has past the retention can't displayed on the web ui. Because the web ui read archived WF info from history store but it will be deleted after the retention. So there is a 404 error.

Yes but shouldn't that change be made in the web ui instead? The api already supports getting archived workflows.

github-actions · 2024-07-10T00:06:51Z

This PR was marked as stale. Please update or close it.

Aoao54 requested a review from a team as a code owner November 15, 2023 11:52

Aoao54 marked this pull request as draft November 16, 2023 06:51

yujunz suggested changes Nov 16, 2023

View reviewed changes

service/frontend/errors.go Outdated Show resolved Hide resolved

Aoao54 force-pushed the support-display-archived-workflow-on-ui branch from 33d46ed to c6d8e5a Compare November 16, 2023 13:40

yujunz reviewed Nov 16, 2023

View reviewed changes

service/frontend/workflow_handler.go Outdated Show resolved Hide resolved

yujunz suggested changes Nov 16, 2023

View reviewed changes

service/frontend/workflow_handler.go Outdated Show resolved Hide resolved

Aoao54 marked this pull request as ready for review November 17, 2023 06:21

Aoao54 force-pushed the support-display-archived-workflow-on-ui branch from c6d8e5a to 140790b Compare November 19, 2023 07:58

Add support for display archived workflow on web ui from s3 archival …

Loading
Loading status checks…

dba6eb1

…store

Aoao54 force-pushed the support-display-archived-workflow-on-ui branch from 140790b to dba6eb1 Compare November 20, 2023 01:41

yycptt reviewed Nov 20, 2023

View reviewed changes

yycptt reviewed Nov 21, 2023

View reviewed changes

gootik mentioned this pull request Jun 28, 2024

Cannot load archived (s3 provider) workflow history on UI #6193

Open

github-actions bot added the stale label Jul 10, 2024

github-actions bot removed the stale label Nov 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for display archived workflow on web ui from s3 archival store #5117

Add support for display archived workflow on web ui from s3 archival store #5117

Aoao54 commented Nov 15, 2023 •

edited by alexshtin

Loading

CLAassistant commented Nov 15, 2023 •

edited

Loading

Aoao54 commented Nov 16, 2023

yycptt left a comment •

edited

Loading

yycptt Nov 20, 2023

Aoao54 Nov 20, 2023

yycptt Nov 21, 2023

Aoao54 Nov 21, 2023

yycptt Nov 20, 2023

Aoao54 Nov 20, 2023

yycptt Nov 21, 2023

yycptt left a comment

yycptt Nov 21, 2023

yycptt Nov 21, 2023

jlegrone commented Dec 18, 2023

jontro commented Mar 10, 2024 •

edited

Loading

Aoao54 commented Mar 11, 2024

jontro commented Mar 11, 2024

github-actions bot commented Jul 10, 2024

Add support for display archived workflow on web ui from s3 archival store #5117

Are you sure you want to change the base?

Add support for display archived workflow on web ui from s3 archival store #5117

Conversation

Aoao54 commented Nov 15, 2023 • edited by alexshtin Loading

CLAassistant commented Nov 15, 2023 • edited Loading

Aoao54 commented Nov 16, 2023

yycptt left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yycptt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlegrone commented Dec 18, 2023

jontro commented Mar 10, 2024 • edited Loading

Aoao54 commented Mar 11, 2024

jontro commented Mar 11, 2024

github-actions bot commented Jul 10, 2024

Aoao54 commented Nov 15, 2023 •

edited by alexshtin

Loading

CLAassistant commented Nov 15, 2023 •

edited

Loading

yycptt left a comment •

edited

Loading

jontro commented Mar 10, 2024 •

edited

Loading