Skip to content

Commit c31f232

Browse files
authored
Fix Reshuffle handling in Prism to recursively remove nested sub-transforms (#38735)
Updates `handleReshuffle` in the Go Prism runner to recursively clean up all nested sub-transforms of a Reshuffle composite transform using `removeSubTransforms`. This prevents nested sub-transforms from being processed in subsequent graph optimization stages.
1 parent 44a2ff6 commit c31f232

2 files changed

Lines changed: 80 additions & 2 deletions

File tree

sdks/go/pkg/beam/runners/prism/internal/handlerunner.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,8 @@ func (h *runner) handleReshuffle(tid string, t *pipepb.PTransform, comps *pipepb
209209
}
210210
}
211211

212-
// And all the sub transforms.
213-
toRemove = append(toRemove, t.GetSubtransforms()...)
212+
// Also recursively remove all sub-transforms.
213+
toRemove = append(toRemove, removeSubTransforms(comps, t.GetSubtransforms())...)
214214

215215
// Return the new components which is the transforms consumer
216216
return prepareResult{
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one or more
2+
// contributor license agreements. See the NOTICE file distributed with
3+
// this work for additional information regarding copyright ownership.
4+
// The ASF licenses this file to You under the Apache License, Version 2.0
5+
// (the "License"); you may not use this file except in compliance with
6+
// the License. You may obtain a copy of the License at
7+
//
8+
// http://www.apache.org/licenses/LICENSE-2.0
9+
//
10+
// Unless required by applicable law or agreed to in writing, software
11+
// distributed under the License is distributed on an "AS IS" BASIS,
12+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
// See the License for the specific language governing permissions and
14+
// limitations under the License.
15+
16+
package internal
17+
18+
import (
19+
"testing"
20+
21+
pipepb "github.com/apache/beam/sdks/v2/go/pkg/beam/model/pipeline_v1"
22+
"github.com/google/go-cmp/cmp"
23+
"google.golang.org/protobuf/testing/protocmp"
24+
)
25+
26+
func TestHandleReshuffle(t *testing.T) {
27+
h := &runner{
28+
config: RunnerCharacteristic{
29+
SDKReshuffle: false,
30+
},
31+
}
32+
33+
comps := &pipepb.Components{
34+
Transforms: map[string]*pipepb.PTransform{
35+
"reshuffle": {
36+
UniqueName: "reshuffle",
37+
Inputs: map[string]string{
38+
"in": "pcol_in",
39+
},
40+
Outputs: map[string]string{
41+
"out": "pcol_out",
42+
},
43+
Subtransforms: []string{"sub_1"},
44+
},
45+
"sub_1": {
46+
UniqueName: "sub_1",
47+
Subtransforms: []string{"sub_2"},
48+
},
49+
"sub_2": {
50+
UniqueName: "sub_2",
51+
},
52+
"consumer": {
53+
UniqueName: "consumer",
54+
Inputs: map[string]string{
55+
"in": "pcol_out",
56+
},
57+
},
58+
},
59+
}
60+
61+
got := h.handleReshuffle("reshuffle", comps.GetTransforms()["reshuffle"], comps)
62+
63+
want := prepareResult{
64+
SubbedComps: nil,
65+
RemovedLeaves: []string{"sub_1", "sub_2"},
66+
ForcedRoots: []string{"consumer"},
67+
}
68+
69+
if d := cmp.Diff(want, got, protocmp.Transform()); d != "" {
70+
t.Errorf("handleReshuffle() diff (-want, +got):\n%v", d)
71+
}
72+
73+
// Verify that the consumer's input has been remapped to the input of the reshuffle
74+
gotInput := comps.GetTransforms()["consumer"].GetInputs()["in"]
75+
if gotInput != "pcol_in" {
76+
t.Errorf("consumer input got %q, want %q", gotInput, "pcol_in")
77+
}
78+
}

0 commit comments

Comments
 (0)