-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Send raw history events from matching to frontend service #8829
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
base: main
Are you sure you want to change the base?
Conversation
Resolved conflicts: - service/history/api/recordworkflowtaskstarted/api.go: kept our return type, added main's GetActiveNamespace parameter - api/matchingservice/v1/request_response.pb.go: regenerated proto files 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add //nolint:staticcheck comments to suppress SA1019 deprecation warnings for RawHistory field usage. These deprecated fields are intentionally used for backward compatibility during rolling upgrades. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…sponseWithRawHistory
- Keep SendRawHistoryBetweenInternalServices default as false - TestQueryNoCurrentPollersButRecentPollers, TestQueryNoRecentPoller: Use emptyClientPollWorkflowTaskQueueResponse for mock client tests since client interface uses PollWorkflowTaskQueueResponse (not the WithRawHistory variant)
yycptt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
synced offline. I think we need a separate flag for this change or we will have a regression on the history service side (an extra decoding)
| // History client will deserialize this message to RecordWorkflowTaskStartedResponse. | ||
| // Deprecated: This field is being replaced by raw_history_bytes which sends raw bytes | ||
| // instead of a proto-decoded History. This avoids matching service having to decode history. | ||
| temporal.api.history.v1.History raw_history = 20 [deprecated = true]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
plz add comment for the plan of deprecating related fields/messages.
| repeated bytes raw_history_bytes = 21; | ||
| } | ||
|
|
||
| // RecordWorkflowTaskStartedResponseWithRawHistory is wire-compatible with RecordWorkflowTaskStartedResponse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like this is no longer used and can be removed?
What changed?
This PR extends the raw history optimization to pass raw history bytes from History Service → Matching Service → Frontend without deserialization in Matching Service.
Key changes:
SendRawHistoryBetweenInternalServicesis enabled, setsRawHistoryBytes(field 21) with raw proto-encoded history batchesPollWorkflowTaskQueueResponseWithRawHistory[][]byte→Historyon the client sideRawHistoryfield (auto-deserialized by gRPC)raw_history_bytes(field 21) toRecordWorkflowTaskStartedResponsePollWorkflowTaskQueueResponseWithRawHistorymessage with wire-compatible layoutraw_history(field 22) toPollWorkflowTaskQueueResponseWhy?
When
history.sendRawHistoryBetweenInternalServicesis enabled, the previous implementation only avoided deserialization from persistence → History Service. However, Matching Service was still deserializing history events (via gRPC auto-deserialization) and re-serializing them when forwarding to Frontend.This change eliminates that unnecessary serialization/deserialization cycle in Matching Service by:
This reduces CPU usage in Matching Service for workflows with large histories.
How did you test it?
tests/workflow_task_test.go)Potential risks
SendRawHistoryBetweenInternalServices must be disabled when rolling back from this version to an older version.