Send raw history events from history to frontend service #7342
Conversation
| // This message must be wire compatible with GetWorkflowExecutionHistoryResponse. | ||
| message GetWorkflowExecutionHistoryResponseWithRaw { | ||
| temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse response = 1; | ||
| repeated bytes history = 2; |
There was a problem hiding this comment.
This is the change from the previous PR. History service will send raw history bytes to this field if config history.sendRawHistoryBetweenInternalService is set to true. Frontend service can check this field and do the modifications like process search attributes, fix the last failed events, filter close events etc and move this history to the correct temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse.History field.
This way it is safe during upgrades and downgrades. We can enable temporal.api.workflowservice.v1.GetWorkflowExecutionHistoryResponse after upgrade has finished. And disable it before downgrading.
| @@ -2376,6 +2376,11 @@ that task will be sent to DLQ.`, | |||
| 0.90, | |||
| "History service health check on persistence error ratio", | |||
| ) | |||
| SendRawHistoryBetweenInternalServices = NewGlobalBoolSetting( | |||
| "history.sendRawHistoryBetweenInternalServices", | |||
| true, | |||
There was a problem hiding this comment.
Setting this to true to verify all functional tests and integration tests passes for this PR. I will set it back to false before merging.
What changed?
Change to send raw history blobs from history service to frontend service. History service returns a new proto message that has a repeated bytes history field.
This response is wire compatible with the original response which has temporal.api.history.v1.History type for this field. This allows history service to not deserialize events from this data blob. This considerably reduces CPU usage.
History service still needs event_id and version decoded from history events. For this we use a new proto message StrippedHistoryEvent which has these two fields only. It takes considerably less CPU to decode events to this struct.
Why?
We have seen incidents of high history CPU usage when large number of GetWorkflowExecutionHistory calls are made to workflows which has large history. With this change we can reduce the CPU burden on history service during this API call.
How did you test it?
Existing unit and functional tests.
Potential risks
Documentation
Is hotfix candidate?