lihjChina commented on PR #10364: URL: https://github.com/apache/seatunnel/pull/10364#issuecomment-3767545490
> * **PhysicalPlan.java:183-185** - `finishedPipelineNum.incrementAndGet()` is called twice for each finished pipeline (also at line 200), causing the condition `finishedPipelineNum == pipelineList.size()` to never become true. Jobs will hang in RUNNING state forever, causing resource leaks. > * **JobConfig.java:52-64** - Adding `pipelineConcurrency` to serialization without versioning breaks backward compatibility. Deserializing old savepoints will read `envOptions` data into `pipelineConcurrency`, causing data corruption and recovery failures. > * **MultipleTableJobConfigParser.java:351-363** - Invalid config values (≤0) only log warnings but don't set a default. While `Integer.MAX_VALUE` is the field default, users expecting concurrency control won't get it without clear error messages. > * **MultipleTableJobConfigParser.java:340-368** - Debug logs like `"Checking for pipeline_concurrency configuration..."` use INFO level, which will flood production logs on every job startup. > * **PhysicalPlan.java:92-94** - `runningPipelineCount` and `nextPipelineIndex` always initialize from 0, ignoring savepoint state. Recovery may exceed concurrency limits or restart completed pipelines, causing data duplication in non-idempotent sinks. > * **PipelineConcurrencyIT.java** - Tests only verify log strings, not actual concurrent behavior. Missing: boundary values (0, negative), savepoint recovery, failure scenarios, unit tests. This is how critical bugs like the double-increment slipped through. > * **EnvCommonOptions.java:114-121; JobConfig.java:37; PhysicalPlan.java:92-94** - Missing JavaDoc for new fields/methods (`pipelineConcurrency`, `runningPipelineCount`, `nextPipelineIndex`, `startNextPipelines()`), reducing maintainability and usability. Thank you very much for your in-depth code review! The issues you identified are all crucial, especially the double increment bug, which, if not fixed, can lead to serious production problems. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
