terrymanu commented on PR #40095: URL: https://github.com/apache/shardingsphere/pull/40095#issuecomment-6035626761
### Result **Review Result: Mergeable** The local change meets the minimal-scope and code-correctness requirements. This conclusion does not establish that the original full-build failure is resolved. ### Evidence - Only `retryIntervalMilliseconds: 5000` was added at `test/it/pipeline/src/test/resources/config_sharding_sphere_jdbc_source.yaml:27`. `maxRetries` retains its default value of 3. - Both affected tests load this configuration through `PipelineContextUtils.initPipelineContextManager()`. The properties reach both the primary context repository and the additional repository initialized by the test utility. - The connection wait in `ZookeeperRepository.initCuratorClient()` increases from 1500ms to 15000ms. The reported downstream delays of 1979ms and 5090ms, together with the explicit user requirement, support this bounded adjustment. The failure has not been reproduced on current upstream code. - The setting also increases Curator’s backoff base and maximum individual backoff delay. Connection failures or lost connections may therefore take longer to report. The 15-second window is not an end-to-end initialization limit and does not resolve disk stalls. - Focused verification: `./mvnw -pl test/it/pipeline -DskipITs -Dspotless.apply.skip=true -Dtest=org.apache.shardingsphere.data.pipeline.core.datasource.PipelineDataSourceManagerTest,org.apache.shardingsphere.data.pipeline.core.util.PipelineDistributedBarrierTest test` exited with code 0: 4 tests, no failures, errors, or skips. - Module verification: `./mvnw -pl test/it/pipeline -Dspotless.apply.skip=true verify` exited with code 0. Surefire reported 122 tests, no failures or errors, and 1 existing skip. All 19 Failsafe tests passed. XML reports confirm that all 11 consumers of the shared initialization ran, including `MigrationJobAPIIT`. ### Coverage - Standalone local candidate based on `ccf56990d75830c7804cc6063963a6f0b37c19e7`. The authorized delta consists of one YAML line. The index remains empty, and both existing untracked directories were preserved. - Reviewed the Pipeline test repository’s connection-wait and backoff configuration from three perspectives: root cause and behavior, affected consumers and contracts, and test/runtime coverage. The final review found no additional issues. - Reuses the existing configuration mechanism without introducing public commitments, architecture changes, dependencies, helper classes, sleeps, or retry wrappers. Defensive behavior and functional/performance effects remain within the explicitly authorized scope. - Spotless: Not Applicable; `pom.xml:1354–1402` does not cover this test YAML. Checkstyle: Not Applicable; `src/resources/checkstyle.xml:22` covers only Java/XML/properties. RAT was not triggered because no files were added and no license headers were changed. - Verified that the copied test resource matches the source configuration and checked the relevant dependency bytecode for consistency. The original full-build command was not provided or executed, so the full-build scenario remains unverified. CI was outside this Code Correctness Review. -- 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]
