Track external payloads stats for workflow execution #8775
Conversation
simvlad
force-pushed
the
simvlad/payload-size-stats-for-workflow-execution
branch
from
9390400 to
c7d356a
Compare
simvlad
force-pushed
the
simvlad/payload-size-stats-for-workflow-execution
branch
2 times, most recently
from
c6237a3 to
dc31bb8
Compare
simvlad
force-pushed
the
simvlad/payload-size-stats-for-workflow-execution
branch
from
dc31bb8 to
ac1098d
Compare
| paginateItems = append(paginateItems, nextBatch) | ||
|
|
||
| // Calculate and accumulate external payload size and count for this batch of history events | ||
| if r.shard.GetConfig().ExternalPayloadsEnabled(namespaceName) { |
There was a problem hiding this comment.
nit: feel like checking the flag once per page/request is also good enough. no strong opinion.
| ms.executionInfo.LastFirstEventTxnId = historyNodeTxnIDs[index] | ||
|
|
||
| // Calculate and add the external payload size and count for this batch | ||
| if ms.config.ExternalPayloadsEnabled(ms.GetNamespaceEntry().Name().String()) { |
There was a problem hiding this comment.
I wonder if we should reset the size & count if the feature flag is disabled.
There was a problem hiding this comment.
We can exclude it from the results of DescribeWorkflowExecution, but not sure if resetting would make this better. We still need to explicitly retrigger the recomputation of it, if it somehow gets computed incorrectly.
There was a problem hiding this comment.
We still need to explicitly retrigger the recomputation of it
How do we know if recomputation is needed for an execution if the feature flag get disabled and then enabled?
There was a problem hiding this comment.
But having those fields set tot 0, is also not a good indicator that the recomputation is necessary.
simvlad
force-pushed
the
simvlad/payload-size-stats-for-workflow-execution
branch
from
93fe315 to
2f2c759
Compare
simvlad
force-pushed
the
simvlad/payload-size-stats-for-workflow-execution
branch
from
2f2c759 to
b67ab26
Compare
## What changed? Added external payload stats (size and count) to DescribeWorkflowExecution. Also change and add functional tests. **This PR depends on Server PR #8775 and API PR temporalio/api#689 ## Why? We'd like to surface the external payload stats when DescribeWorkflowExecution is called to see how many and what size of external payloads the workflow is referencing. Numbers are coming from the workflow mutable state. This is similar to how the HistorySize is kept. ## How did you test it? - [X] built - [ ] run locally and tested manually - [X] covered by existing tests - [X] added new unit test(s) - [X] added new functional test(s)
What changed?
Keep the total number and the size of the external payloads per the workflow execution
Why?
We are working on building the support for external payloads in SDK, which are stored outside of Temporal. We'd like to be able to show the total size and the number of external payloads in the given workflow execution.
How did you test it?
Potential risks
N/A