Re: [PR] [GLUTEN-12468][FLINK] Handle WatermarkStatus elements in GlutenSourceFunction [gluten]

["KevinyhZou (via GitHub)" <gi...@apache.org>]

KevinyhZou commented on code in PR #12461:
URL: https://github.com/apache/gluten/pull/12461#discussion_r3575995302


##########
gluten-flink/runtime/src/main/java/org/apache/gluten/table/runtime/operators/GlutenSourceFunction.java:
##########


Review Comment:
   **Please tighten / 收敛 `shouldCallNoMoreSplits`**
   
   This 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:
   
   ```java
   public void setShouldCallNoMoreSplits(boolean value) { ... }
   public boolean isShouldCallNoMoreSplits() { ... }
   ```
   
   and it is also propagated through `OffloadedJobGraphGenerator` / `GlutenStreamSource`.
   
   Risks:
   1. Accidental `false` in production changes split lifecycle semantics.
   2. Offload path now has to carry a UT-oriented toggle as a first-class field.
   3. Naming/`setter` surface suggests general configuration, but docs say it is for “unbounded streaming **test** scenarios”.
   
   **Suggestions (any one is fine):**
   - Prefer a constructor param or package-private / `@VisibleForTesting` API instead of a public setter.
   - Avoid wiring this through `OffloadedJobGraphGenerator` unless production offload truly needs a non-default value (default should stay `true`).
   - Longer term: derive from connector/split boundedness (unbounded Kafka → don’t auto `noMoreSplits`) rather than a hand-set boolean.
   
   Functionality can stay; please just shrink visibility and keep it out of the general runtime surface.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org