Skip to content

Commit 0f78c5e

Browse files
authored
refactor(engine): simplify task data shape (#5198)
* refactor: simplify the V1StepRunData and read error messages out of the task events * feat: add new columns to db * chore: gen * feat: wiring * refactor: simplify a bit * fix: migration version * fix: handle replay bug * fix: add an env var to gate * fix: simplify by adding a payload version * fix: simplify more * fix: simplify yet more, remove some tests * fix: make filter payloads compatible across versions * chore: shrink diff more
1 parent b80da9a commit 0f78c5e

20 files changed

Lines changed: 641 additions & 214 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
-- +goose Up
2+
-- +goose StatementBegin
3+
ALTER TABLE v1_task
4+
ADD COLUMN triggers JSONB,
5+
ADD COLUMN dag_parent_task_run_ids UUID[],
6+
ADD COLUMN child_task_desired_worker_labels JSONB,
7+
ADD COLUMN task_input_payload_version SMALLINT NOT NULL DEFAULT 1;
8+
-- +goose StatementEnd
9+
10+
-- +goose Down
11+
-- +goose StatementBegin
12+
ALTER TABLE v1_task
13+
DROP COLUMN triggers,
14+
DROP COLUMN dag_parent_task_run_ids,
15+
DROP COLUMN child_task_desired_worker_labels,
16+
DROP COLUMN task_input_payload_version;
17+
-- +goose StatementEnd

‎internal/services/dispatcher/dispatcher.go‎

Lines changed: 42 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -810,13 +810,11 @@ func (d *DispatcherImpl) populateTaskData(
810810
input = task.Input
811811
}
812812

813-
currInput := &v1.V1StepRunData{}
813+
currInput, err := v1.NewV1StepRunDataFromTaskAndInputPayload(task, input)
814814

815-
if input != nil {
816-
if err := json.Unmarshal(input, currInput); err != nil {
817-
d.l.Warn().Ctx(ctx).Err(err).Msg("failed to unmarshal input")
818-
continue
819-
}
815+
if err != nil {
816+
d.l.Warn().Ctx(ctx).Err(err).Msg("failed to unmarshal input")
817+
continue
820818
}
821819

822820
if len(currInput.DagParentTaskRunIds) > 0 {
@@ -826,24 +824,13 @@ func (d *DispatcherImpl) populateTaskData(
826824
dagParentExternalIdSet[parentExternalId] = struct{}{}
827825
}
828826
} else if parentData, ok := parentDataMap[task.ID]; ok {
829-
readableIdToData := make(map[string]map[string]interface{})
830-
831-
for _, outputEvent := range parentData {
832-
outputMap := make(map[string]interface{})
833-
834-
if len(outputEvent.Output) > 0 {
835-
if err := json.Unmarshal(outputEvent.Output, &outputMap); err != nil {
836-
d.l.Warn().Ctx(ctx).Err(err).Msg("failed to unmarshal output")
837-
continue
838-
}
839-
}
840-
841-
readableIdToData[outputEvent.StepReadableID] = outputMap
842-
}
827+
parentOutputs, stepRunErrors := splitParentOutputsAndStepRunErrors(ctx, d.l, task.ID, parentData)
843828

844-
currInput.Parents = readableIdToData
845-
inputs[payloadKey] = currInput.Bytes()
829+
currInput.Parents = parentOutputs
830+
currInput.StepRunErrors = stepRunErrors
846831
}
832+
833+
inputs[payloadKey] = currInput.Bytes()
847834
}
848835

849836
if len(dagChildInputs) > 0 {
@@ -921,6 +908,39 @@ func (d *DispatcherImpl) populateTaskData(
921908
return taskIdToData, nil
922909
}
923910

911+
func splitParentOutputsAndStepRunErrors(
912+
ctx context.Context,
913+
l *zerolog.Logger,
914+
taskId int64,
915+
terminalEvents []*v1.TaskOutputEvent,
916+
) (map[string]map[string]interface{}, map[string]string) {
917+
parentOutputs := make(map[string]map[string]interface{})
918+
stepRunErrors := make(map[string]string)
919+
920+
for _, event := range terminalEvents {
921+
if event.IsFailed() {
922+
if event.TaskId != taskId {
923+
stepRunErrors[event.StepReadableID] = event.ErrorMessage
924+
}
925+
926+
continue
927+
}
928+
929+
outputMap := make(map[string]interface{})
930+
931+
if len(event.Output) > 0 {
932+
if err := json.Unmarshal(event.Output, &outputMap); err != nil {
933+
l.Warn().Ctx(ctx).Err(err).Msg("failed to unmarshal output")
934+
continue
935+
}
936+
}
937+
938+
parentOutputs[event.StepReadableID] = outputMap
939+
}
940+
941+
return parentOutputs, stepRunErrors
942+
}
943+
924944
// dagChildTaskInput is a durable DAG-operator child task awaiting parent output resolution:
925945
// its own payload key (to write the resolved input back into `inputs`) plus its unmarshaled
926946
// input (which carries the list of parent external IDs it needs outputs for).

‎pkg/config/loader/loader.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,7 @@ func (c *ConfigLoader) InitDataLayer() (res *database.Layer, err error) {
435435
ExternalCutoverNumConcurrentOffloads: scf.PayloadStore.ExternalCutoverNumConcurrentOffloads,
436436
InlineStoreTTL: &inlineStoreTTL,
437437
EnableWindowSizeOptimization: scf.PayloadStore.EnableWindowSizeOptimization,
438+
WrapTaskInputInStepRunData: scf.PayloadStore.WrapTaskInputInStepRunData,
438439
}
439440

440441
statusUpdateOpts := repov1.StatusUpdateBatchSizeLimits{

‎pkg/config/server/server.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -826,6 +826,12 @@ type PayloadStoreConfig struct {
826826
ExternalCutoverNumConcurrentOffloads int32 `mapstructure:"externalCutoverNumConcurrentOffloads" json:"externalCutoverNumConcurrentOffloads,omitempty" default:"10"`
827827
InlineStoreTTLDays int32 `mapstructure:"inlineStoreTTLDays" json:"inlineStoreTTLDays,omitempty" default:"2"`
828828
EnableWindowSizeOptimization bool `mapstructure:"enableWindowSizeOptimization" json:"enableWindowSizeOptimization,omitempty" default:"true"`
829+
// Writes the task input to the payload store wrapped in the full step run data, which engines before the
830+
// `triggers`, `dag_parent_task_run_ids` and `child_task_desired_worker_labels` columns on v1_task can read.
831+
// Every engine from this version on reads both shapes. Turn off only once every engine is on this version
832+
// or later, because an older engine cannot read a bare input and turning it back on does not repair rows
833+
// already written bare.
834+
WrapTaskInputInStepRunData bool `mapstructure:"wrapTaskInputInStepRunData" json:"wrapTaskInputInStepRunData,omitempty" default:"true"`
829835
}
830836

831837
func (c *ServerConfig) HasService(name string) bool {
@@ -1120,6 +1126,7 @@ func BindAllEnv(v *viper.Viper) {
11201126
_ = v.BindEnv("payloadStore.externalCutoverNumConcurrentOffloads", "SERVER_PAYLOAD_STORE_EXTERNAL_CUTOVER_NUM_CONCURRENT_OFFLOADS")
11211127
_ = v.BindEnv("payloadStore.inlineStoreTTLDays", "SERVER_PAYLOAD_STORE_INLINE_STORE_TTL_DAYS")
11221128
_ = v.BindEnv("payloadStore.enableWindowSizeOptimization", "SERVER_PAYLOAD_STORE_ENABLE_WINDOW_SIZE_OPTIMIZATION")
1129+
_ = v.BindEnv("payloadStore.wrapTaskInputInStepRunData", "SERVER_PAYLOAD_STORE_WRAP_TASK_INPUT_IN_STEP_RUN_DATA")
11231130

11241131
// cron operations options
11251132
_ = v.BindEnv("cronOperations.taskAnalyzeCronInterval", "SERVER_CRON_OPERATIONS_TASK_ANALYZE_CRON_INTERVAL")

‎pkg/repository/input.go‎

Lines changed: 170 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package repository
22

33
import (
44
"encoding/json"
5+
"fmt"
56

67
"github.com/google/uuid"
78

@@ -35,12 +36,35 @@ func (s *sharedRepository) DesiredWorkerId(t *TaskInput) *uuid.UUID {
3536
return nil
3637
}
3738

38-
func (s *sharedRepository) newTaskInputFromExistingBytes(inputBytes []byte) *TaskInput {
39+
func (s *sharedRepository) newTaskInputFromTaskInputPayload(version taskInputPayloadVersion, taskInputPayload []byte) *TaskInput {
3940
i := &TaskInput{}
4041

41-
err := json.Unmarshal(inputBytes, i)
42+
var userInput []byte
4243

43-
if err != nil {
44+
switch version {
45+
case taskInputPayloadVersionStepRunData:
46+
stepRunData, err := stepRunDataFromVersion1TaskInputPayload(taskInputPayload)
47+
48+
if err != nil {
49+
s.l.Error().Err(err).Msg("failed to unmarshal step run data")
50+
return i
51+
}
52+
53+
userInput = stepRunData.Input
54+
i.DagParentTaskRunIds = stepRunData.DagParentTaskRunIds
55+
i.DesiredWorkerLabels = stepRunData.DesiredWorkerLabels
56+
case taskInputPayloadVersionUserInputOnly:
57+
userInput = taskInputPayload
58+
default:
59+
s.l.Error().Msgf("unknown task input payload version %d", version)
60+
return i
61+
}
62+
63+
if len(userInput) == 0 {
64+
return i
65+
}
66+
67+
if err := json.Unmarshal(userInput, &i.Input); err != nil {
4468
s.l.Error().Err(err).Msg("failed to unmarshal input bytes")
4569
}
4670

@@ -126,9 +150,14 @@ func (s *sharedRepository) ToV1StepRunData(t *TaskInput) *V1StepRunData {
126150

127151
triggers["filter_payload"] = t.FilterPayload
128152

153+
input, err := json.Marshal(t.Input)
154+
155+
if err != nil {
156+
s.l.Warn().Err(err).Msg("failed to marshal input")
157+
}
158+
129159
return &V1StepRunData{
130-
Input: t.Input,
131-
TriggeredBy: "manual",
160+
Input: input,
132161
Parents: parents,
133162
Triggers: triggers,
134163
StepRunErrors: stepRunErrors,
@@ -137,36 +166,104 @@ func (s *sharedRepository) ToV1StepRunData(t *TaskInput) *V1StepRunData {
137166
}
138167
}
139168

140-
func (r *sharedRepository) V1StepRunDataFromBytes(b []byte) (*V1StepRunData, error) {
141-
if len(b) == 0 {
142-
return nil, nil
169+
type taskInputPayloadVersion int16
170+
171+
const (
172+
taskInputPayloadVersionStepRunData taskInputPayloadVersion = 1
173+
taskInputPayloadVersionUserInputOnly taskInputPayloadVersion = 2
174+
)
175+
176+
func (s *sharedRepository) taskInputPayloadVersionToWrite() taskInputPayloadVersion {
177+
if s.wrapTaskInputInStepRunData {
178+
return taskInputPayloadVersionStepRunData
143179
}
144180

145-
out := &V1StepRunData{}
181+
return taskInputPayloadVersionUserInputOnly
182+
}
146183

147-
err := json.Unmarshal(b, out)
184+
func stepRunDataFromVersion1TaskInputPayload(taskInputPayload []byte) (*V1StepRunData, error) {
185+
stepRunData := &V1StepRunData{}
148186

149-
if err != nil {
187+
if len(taskInputPayload) == 0 {
188+
return stepRunData, nil
189+
}
190+
191+
if err := json.Unmarshal(taskInputPayload, stepRunData); err != nil {
150192
return nil, err
151193
}
152194

153-
return out, nil
195+
return stepRunData, nil
154196
}
155197

156-
type V1StepRunData struct {
157-
Input map[string]interface{} `json:"input"`
158-
TriggeredBy string `json:"triggered_by"`
159-
Parents map[string]map[string]interface{} `json:"parents"`
198+
func userInputFromTaskInputPayload(version taskInputPayloadVersion, taskInputPayload []byte) ([]byte, error) {
199+
switch version {
200+
case taskInputPayloadVersionStepRunData:
201+
stepRunData, err := stepRunDataFromVersion1TaskInputPayload(taskInputPayload)
160202

161-
Triggers map[string]map[string]interface{} `json:"triggers"`
203+
if err != nil {
204+
return nil, fmt.Errorf("failed to unmarshal step run data: %w", err)
205+
}
206+
207+
return stepRunData.Input, nil
208+
case taskInputPayloadVersionUserInputOnly:
209+
return taskInputPayload, nil
210+
default:
211+
return nil, fmt.Errorf("unknown task input payload version %d", version)
212+
}
213+
}
214+
215+
func NewV1StepRunDataFromTaskAndInputPayload(task *sqlcv1.V1Task, taskInputPayload []byte) (*V1StepRunData, error) {
216+
switch version := taskInputPayloadVersion(task.TaskInputPayloadVersion); version {
217+
case taskInputPayloadVersionStepRunData:
218+
return stepRunDataFromVersion1TaskInputPayload(taskInputPayload)
219+
case taskInputPayloadVersionUserInputOnly:
220+
return newV1StepRunDataFromTaskAndUserInput(task, taskInputPayload)
221+
default:
222+
return nil, fmt.Errorf("unknown task input payload version %d", version)
223+
}
224+
}
225+
226+
// newV1StepRunDataFromTaskAndUserInput leaves Parents empty and StepRunErrors unset. The dispatcher fills both in
227+
// from the task events of the task's parents.
228+
func newV1StepRunDataFromTaskAndUserInput(task *sqlcv1.V1Task, userInput []byte) (*V1StepRunData, error) {
229+
triggers := map[string]map[string]interface{}{"filter_payload": nil}
230+
231+
if len(task.Triggers) > 0 {
232+
if err := json.Unmarshal(task.Triggers, &triggers); err != nil {
233+
return nil, fmt.Errorf("failed to unmarshal triggers: %w", err)
234+
}
235+
}
236+
237+
var childTaskDesiredWorkerLabels []*sqlcv1.GetDesiredLabelsRow
238+
239+
if len(task.ChildTaskDesiredWorkerLabels) > 0 {
240+
if err := json.Unmarshal(task.ChildTaskDesiredWorkerLabels, &childTaskDesiredWorkerLabels); err != nil {
241+
return nil, fmt.Errorf("failed to unmarshal child task desired worker labels: %w", err)
242+
}
243+
}
244+
245+
input := json.RawMessage(userInput)
162246

163-
// custom-set user data for the step
164-
UserData map[string]interface{} `json:"user_data"`
247+
if len(input) == 0 {
248+
input = json.RawMessage("{}")
249+
}
250+
251+
return &V1StepRunData{
252+
Input: input,
253+
Parents: make(map[string]map[string]interface{}),
254+
Triggers: triggers,
255+
DagParentTaskRunIds: task.DagParentTaskRunIds,
256+
DesiredWorkerLabels: childTaskDesiredWorkerLabels,
257+
}, nil
258+
}
165259

166-
// overrides set from the playground
167-
Overrides map[string]interface{} `json:"overrides"`
260+
type V1StepRunData struct {
261+
Input json.RawMessage `json:"input"`
262+
Parents map[string]map[string]interface{} `json:"parents"`
263+
Triggers map[string]map[string]interface{} `json:"triggers"`
168264

169-
// errors in upstream steps (only used in on-failure step)
265+
// errors in upstream steps. Set at task creation for engines that read the wrapped payload, and overwritten
266+
// from the parents' task events when the task is dispatched.
170267
StepRunErrors map[string]string `json:"step_run_errors,omitempty"`
171268

172269
// task run external IDs of parent tasks in a durable DAG orchestration
@@ -177,25 +274,67 @@ type V1StepRunData struct {
177274
DesiredWorkerLabels []*sqlcv1.GetDesiredLabelsRow `json:"desired_worker_labels,omitempty"`
178275
}
179276

180-
func (v1 *V1StepRunData) Bytes() []byte {
181-
out, err := json.Marshal(v1)
277+
func (v1 *V1StepRunData) taskInputPayload(version taskInputPayloadVersion) []byte {
278+
switch version {
279+
case taskInputPayloadVersionStepRunData:
280+
return v1.Bytes()
281+
case taskInputPayloadVersionUserInputOnly:
282+
return v1.userInput()
283+
default:
284+
panic(fmt.Sprintf("unknown task input payload version %d", version))
285+
}
286+
}
182287

183-
if err != nil {
184-
return []byte("{}")
288+
// userInput is null for a task created without an input, which matches what Bytes returns for it.
289+
func (v1 *V1StepRunData) userInput() []byte {
290+
if v1 == nil {
291+
return []byte("null")
185292
}
186293

187-
return out
294+
return v1.Input
295+
}
296+
297+
type stepRunDataTaskColumns struct {
298+
triggers []byte
299+
dagParentTaskRunIds []byte
300+
childTaskDesiredWorkerLabels []byte
188301
}
189302

190-
func (v1 *V1StepRunData) InputBytes() []byte {
303+
func (v1 *V1StepRunData) taskColumns() (stepRunDataTaskColumns, error) {
304+
columns := stepRunDataTaskColumns{}
305+
191306
if v1 == nil {
192-
return nil
307+
return columns, nil
308+
}
309+
310+
var err error
311+
312+
if len(v1.Triggers) > 0 {
313+
if columns.triggers, err = json.Marshal(v1.Triggers); err != nil {
314+
return columns, fmt.Errorf("failed to marshal triggers: %w", err)
315+
}
193316
}
194317

195-
out, err := json.Marshal(v1.Input)
318+
if len(v1.DagParentTaskRunIds) > 0 {
319+
if columns.dagParentTaskRunIds, err = json.Marshal(v1.DagParentTaskRunIds); err != nil {
320+
return columns, fmt.Errorf("failed to marshal dag parent task run ids: %w", err)
321+
}
322+
}
323+
324+
if len(v1.DesiredWorkerLabels) > 0 {
325+
if columns.childTaskDesiredWorkerLabels, err = json.Marshal(v1.DesiredWorkerLabels); err != nil {
326+
return columns, fmt.Errorf("failed to marshal child task desired worker labels: %w", err)
327+
}
328+
}
329+
330+
return columns, nil
331+
}
332+
333+
func (v1 *V1StepRunData) Bytes() []byte {
334+
out, err := json.Marshal(v1)
196335

197336
if err != nil {
198-
return nil
337+
return []byte("{}")
199338
}
200339

201340
return out

0 commit comments

Comments
 (0)