-
Notifications
You must be signed in to change notification settings - Fork 625
[GLUTEN-12468][FLINK] Handle WatermarkStatus elements in GlutenSourceFunction #12461
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
Open
lgbo-ustc
wants to merge
15
commits into
apache:main
Choose a base branch
from
bigo-sg:feature/idle-source-handling
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
abce609
fix: Handle WatermarkStatus elements in GlutenSourceFunction
lgbo-ustc 3bcf527
test: Add unit tests for GlutenSourceFunction WatermarkStatus handling
lgbo-ustc 53d7c08
test: Add integration test for idle watermark status in WatermarkAssi…
lgbo-ustc 15c246d
fix: Update velox4j reference to feature/idle-source-handling branch
lgbo-ustc ec01991
fix: Add shouldCallNoMoreSplits option to GlutenSourceFunction for un…
lgbo-ustc bf1aa13
feat: E2E test for idle WatermarkStatus detection with Kafka + MiniCl…
lgbo-ustc a2ab1ae
chore: remove obsolete GlutenSourceFunctionWatermarkStatusTest
lgbo-ustc c8d9460
test: verify idle inputs are excluded from combined min-watermark
lgbo-ustc 9cd1b66
fix: upgrade surefire in gluten-flink-ut from 3.0.0-M5 to 3.3.0
lgbo-ustc 8e9685a
fix: revert gluten-flink surefire upgrade
lgbo-ustc 23d6308
fix: clean up idle source E2E test
lgbo-ustc e231c12
Update velox4j reference for idle source handling
lgbo-ustc 4f2a208
fix: Remove noMoreSplits test toggle
lgbo-ustc c1301db
Update velox4j reference for watermark destructor fix
lgbo-ustc 9c9d1e9
Update velox4j reference for idle timer tests
lgbo-ustc File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
241 changes: 241 additions & 0 deletions
241
...va/org/apache/gluten/streaming/api/operators/GlutenOneInputWatermarkAssignerIdleTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.gluten.streaming.api.operators; | ||
|
|
||
| import org.apache.gluten.rexnode.RexConversionContext; | ||
| import org.apache.gluten.rexnode.RexNodeConverter; | ||
| import org.apache.gluten.rexnode.Utils; | ||
| import org.apache.gluten.table.runtime.operators.GlutenOneInputOperator; | ||
| import org.apache.gluten.table.runtime.stream.common.Velox4jEnvironment; | ||
| import org.apache.gluten.util.LogicalTypeConverter; | ||
| import org.apache.gluten.util.PlanNodeIdGenerator; | ||
|
|
||
| import io.github.zhztheplayer.velox4j.expression.TypedExpr; | ||
| import io.github.zhztheplayer.velox4j.plan.EmptyNode; | ||
| import io.github.zhztheplayer.velox4j.plan.ProjectNode; | ||
| import io.github.zhztheplayer.velox4j.plan.StatefulPlanNode; | ||
| import io.github.zhztheplayer.velox4j.plan.WatermarkAssignerNode; | ||
|
|
||
| import org.apache.flink.api.common.serialization.SerializerConfigImpl; | ||
| import org.apache.flink.api.common.typeinfo.TypeInformation; | ||
| import org.apache.flink.streaming.api.watermark.Watermark; | ||
| import org.apache.flink.streaming.runtime.streamrecord.StreamRecord; | ||
| import org.apache.flink.streaming.runtime.watermarkstatus.WatermarkStatus; | ||
| import org.apache.flink.streaming.util.OneInputStreamOperatorTestHarness; | ||
| import org.apache.flink.table.data.GenericRowData; | ||
| import org.apache.flink.table.data.RowData; | ||
| import org.apache.flink.table.planner.calcite.FlinkRexBuilder; | ||
| import org.apache.flink.table.planner.calcite.FlinkTypeFactory; | ||
| import org.apache.flink.table.planner.calcite.FlinkTypeSystem; | ||
| import org.apache.flink.table.runtime.typeutils.InternalTypeInfo; | ||
| import org.apache.flink.table.types.logical.BigIntType; | ||
| import org.apache.flink.table.types.logical.IntType; | ||
| import org.apache.flink.table.types.logical.LogicalType; | ||
| import org.apache.flink.table.types.logical.RowType; | ||
|
|
||
| import org.apache.calcite.rex.RexNode; | ||
| import org.apache.calcite.sql.type.SqlTypeName; | ||
| import org.junit.jupiter.api.BeforeAll; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Queue; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
|
|
||
| /** | ||
| * Integration test for WatermarkAssigner idle detection. | ||
| * | ||
| * <p>Native {@code checkWatermarkStatus} is driven by the {@code next()} → {@code advance()} loop | ||
| * inside the {@code GlutenOneInputOperator}'s drain pipeline. Since {@code addInput()} resets the | ||
| * idle baseline on every record, the idle check must happen on an {@code advance()} call that | ||
| * processes no new input. We achieve this by calling {@code processWatermark()} (which triggers a | ||
| * drain cycle without new data) after waiting past the idle timeout. | ||
| */ | ||
| public class GlutenOneInputWatermarkAssignerIdleTest { | ||
|
|
||
| private static FlinkTypeFactory typeFactory; | ||
| private static FlinkRexBuilder rexBuilder; | ||
| private static RowType inputFlinkRowType; | ||
| private static io.github.zhztheplayer.velox4j.type.RowType inputVeloxType; | ||
| private static TypeInformation<RowData> typeInfo; | ||
|
|
||
| @BeforeAll | ||
| static void setUpClass() { | ||
| Velox4jEnvironment.initializeOnce(); | ||
|
|
||
| typeFactory = | ||
| new FlinkTypeFactory( | ||
| Thread.currentThread().getContextClassLoader(), FlinkTypeSystem.INSTANCE); | ||
| rexBuilder = new FlinkRexBuilder(typeFactory); | ||
|
|
||
| inputFlinkRowType = | ||
| RowType.of(new LogicalType[] {new IntType(), new BigIntType()}, new String[] {"id", "ts"}); | ||
| inputVeloxType = | ||
| (io.github.zhztheplayer.velox4j.type.RowType) | ||
| LogicalTypeConverter.toVLType(inputFlinkRowType); | ||
| typeInfo = InternalTypeInfo.of(inputFlinkRowType); | ||
| } | ||
|
|
||
| @Test | ||
| void testIdleDetectionWithRealTimePassage() throws Exception { | ||
| long idleTimeout = 100L; // 100 ms | ||
| long watermarkInterval = 50L; | ||
|
|
||
| // ── Watermark expression: reference the ts field (index 1) ── | ||
| List<String> fieldNames = Utils.getNamesFromRowType(inputFlinkRowType); | ||
| RexNode tsRef = rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.BIGINT), 1); | ||
| TypedExpr watermarkExpr = | ||
| RexNodeConverter.toTypedExpr(tsRef, new RexConversionContext(fieldNames)); | ||
|
|
||
| ProjectNode watermarkProject = | ||
| new ProjectNode( | ||
| PlanNodeIdGenerator.newId(), | ||
| List.of(new EmptyNode(inputVeloxType)), | ||
| List.of("TIMESTAMP"), | ||
| List.of(watermarkExpr)); | ||
|
|
||
| // ── WatermarkAssignerNode ── | ||
| WatermarkAssignerNode assignerNode = | ||
| new WatermarkAssignerNode( | ||
| PlanNodeIdGenerator.newId(), | ||
| null, | ||
| watermarkProject, | ||
| idleTimeout, | ||
| 1, // rowtimeFieldIndex (ts at index 1) | ||
| watermarkInterval); | ||
|
|
||
| // ── GlutenOneInputOperator ── | ||
| GlutenOneInputOperator<RowData, RowData> operator = | ||
| new GlutenOneInputOperator<>( | ||
| new StatefulPlanNode(assignerNode.getId(), assignerNode), | ||
| PlanNodeIdGenerator.newId(), | ||
| inputVeloxType, | ||
| Map.of(assignerNode.getId(), inputVeloxType), | ||
| RowData.class, | ||
| RowData.class, | ||
| "IdleDetectionTest"); | ||
|
|
||
| // ── Test harness ── | ||
| OneInputStreamOperatorTestHarness<RowData, RowData> harness = | ||
| new OneInputStreamOperatorTestHarness<>( | ||
| operator, typeInfo.createSerializer(new SerializerConfigImpl())); | ||
| harness.setup(typeInfo.createSerializer(new SerializerConfigImpl())); | ||
| harness.open(); | ||
|
|
||
| try { | ||
| // ── Phase 1: feed one record ── | ||
| GenericRowData record1 = GenericRowData.of(1, 1000L); | ||
| harness.processElement(new StreamRecord<>(record1, 1000L)); | ||
| // output: StreamRecord(record1), Watermark(1000) | ||
| // After drain, checkWatermarkStatus(now1) scheduled timer at now1+100ms. | ||
|
|
||
| // ── Phase 2: wait past idleTimeout, then trigger a drain WITHOUT new input ── | ||
| Thread.sleep(idleTimeout * 2); // 200 ms > 100 ms | ||
| harness.processWatermark(new Watermark(0)); | ||
| // Inside drain: advance → next → advanceWithFuture → blocked | ||
| // → checkWatermarkStatus(now2) → idle detected (now2 - lastRecordTime > 100ms) | ||
| // → push WatermarkStatus.IDLE to pendings_ | ||
|
|
||
| // ── Phase 3: feed a second record — the drain first pops pending IDLE ── | ||
| GenericRowData record2 = GenericRowData.of(2, 2000L); | ||
| harness.processElement(new StreamRecord<>(record2, 2000L)); | ||
| // drain: advance → next → pendings_ non-empty (IDLE) → pop IDLE → emit IDLE | ||
| // → advance → next → process record2 → addInput → onRecord → idle was true | ||
| // → emit ACTIVE → push ACTIVE → advance → push record2 + watermark | ||
| // → pop ACTIVE → emit ACTIVE → pop record2 → collect → pop watermark → emit | ||
|
|
||
| // ── Assertions ── | ||
| Queue<Object> output = harness.getOutput(); | ||
| assertThat(output) | ||
| .as("Output must contain WatermarkStatus.IDLE after idle timeout") | ||
| .anyMatch(e -> e instanceof WatermarkStatus && ((WatermarkStatus) e).isIdle()); | ||
| assertThat(output) | ||
| .as("Output must contain WatermarkStatus.ACTIVE after idle→active transition") | ||
| .anyMatch(e -> e instanceof WatermarkStatus && !((WatermarkStatus) e).isIdle()); | ||
| assertThat(output) | ||
| .as("All input records must be preserved") | ||
| .anyMatch( | ||
| e -> | ||
| e instanceof StreamRecord | ||
| && ((StreamRecord<RowData>) e).getValue().getInt(0) == 1) | ||
| .anyMatch( | ||
| e -> | ||
| e instanceof StreamRecord | ||
| && ((StreamRecord<RowData>) e).getValue().getInt(0) == 2); | ||
| } finally { | ||
| harness.close(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void testNoIdleWithContinuousRecords() throws Exception { | ||
| long idleTimeout = 100L; | ||
| long watermarkInterval = 50L; | ||
|
|
||
| List<String> fieldNames = Utils.getNamesFromRowType(inputFlinkRowType); | ||
| RexNode tsRef = rexBuilder.makeInputRef(typeFactory.createSqlType(SqlTypeName.BIGINT), 1); | ||
| TypedExpr watermarkExpr = | ||
| RexNodeConverter.toTypedExpr(tsRef, new RexConversionContext(fieldNames)); | ||
|
|
||
| ProjectNode watermarkProject = | ||
| new ProjectNode( | ||
| PlanNodeIdGenerator.newId(), | ||
| List.of(new EmptyNode(inputVeloxType)), | ||
| List.of("TIMESTAMP"), | ||
| List.of(watermarkExpr)); | ||
|
|
||
| WatermarkAssignerNode assignerNode = | ||
| new WatermarkAssignerNode( | ||
| PlanNodeIdGenerator.newId(), null, watermarkProject, idleTimeout, 1, watermarkInterval); | ||
|
|
||
| GlutenOneInputOperator<RowData, RowData> operator = | ||
| new GlutenOneInputOperator<>( | ||
| new StatefulPlanNode(assignerNode.getId(), assignerNode), | ||
| PlanNodeIdGenerator.newId(), | ||
| inputVeloxType, | ||
| Map.of(assignerNode.getId(), inputVeloxType), | ||
| RowData.class, | ||
| RowData.class, | ||
| "NoIdleTest"); | ||
|
|
||
| OneInputStreamOperatorTestHarness<RowData, RowData> harness = | ||
| new OneInputStreamOperatorTestHarness<>( | ||
| operator, typeInfo.createSerializer(new SerializerConfigImpl())); | ||
| harness.setup(typeInfo.createSerializer(new SerializerConfigImpl())); | ||
| harness.open(); | ||
|
|
||
| try { | ||
| GenericRowData record1 = GenericRowData.of(1, 1000L); | ||
| GenericRowData record2 = GenericRowData.of(2, 2000L); | ||
|
|
||
| harness.processElement(new StreamRecord<>(record1, 1000L)); | ||
| // Feed second record immediately (no idle gap) → addInput resets baseline | ||
| harness.processElement(new StreamRecord<>(record2, 2000L)); | ||
| // Then drain without new data — idle timeout has NOT elapsed since record2 | ||
| harness.processWatermark(new Watermark(0)); | ||
|
|
||
| Queue<Object> output = harness.getOutput(); | ||
| assertThat(output) | ||
| .as("No WatermarkStatus.IDLE should appear when records arrive continuously") | ||
| .noneMatch(e -> e instanceof WatermarkStatus && ((WatermarkStatus) e).isIdle()); | ||
| } finally { | ||
| harness.close(); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Please tighten / 收敛
shouldCallNoMoreSplitsThis flag is introduced mainly so the idle E2E test can keep the native task alive (skip
task.noMoreSplits(id)after the initial split). That need is valid, but the current shape turns a test knob into a public production API:and it is also propagated through
OffloadedJobGraphGenerator/GlutenStreamSource.Risks:
falsein production changes split lifecycle semantics.settersurface suggests general configuration, but docs say it is for “unbounded streaming test scenarios”.Suggestions (any one is fine):
@VisibleForTestingAPI instead of a public setter.OffloadedJobGraphGeneratorunless production offload truly needs a non-default value (default should staytrue).noMoreSplits) rather than a hand-set boolean.Functionality can stay; please just shrink visibility and keep it out of the general runtime surface.