sandynz commented on PR #2530:
URL:
https://github.com/apache/shardingsphere-elasticjob/pull/2530#issuecomment-5111854205
### Summary
**Review Result: Mergeable**
Reason: The latest commit correctly fixes the lifecycle server-status
stale-read path. This result covers code scope only; CI not reviewed by request.
### Evidence
- `JobStatisticsAPIImpl` now uses authoritative reads for both all-server
status aggregation and per-IP status queries:
-
`lifecycle/src/main/java/org/apache/shardingsphere/elasticjob/lifecycle/internal/statistics/JobStatisticsAPIImpl.java:101`
-
`lifecycle/src/main/java/org/apache/shardingsphere/elasticjob/lifecycle/internal/statistics/JobStatisticsAPIImpl.java:150`
- `ServerStatisticsAPIImpl` uses the authoritative value when calculating
`disabledJobsNum`:
-
`lifecycle/src/main/java/org/apache/shardingsphere/elasticjob/lifecycle/internal/statistics/ServerStatisticsAPIImpl.java:62`
- The regression tests distinguish stale cached `ENABLED` values from
direct `DISABLED` values, covering all three changed read paths.
- The change preserves `CoordinatorRegistryCenter`, provider cache, kernel
scheduling, API/SPI, configuration, and distribution contracts.
- Both ZooKeeper and etcd implement the existing `getDirectly()` contract.
Missing or concurrently deleted server nodes remain null-safe.
- No unrelated changes, unnecessary abstractions, or whitespace-only
expansion were found.
### Review Details
- Review Focus: Code Correctness Review. CI not reviewed by request.
- Reviewed Scope:
- Head: `7be67238322ced36de1fab6f80a4a98af1f7b0f0`
- Merge base: `0694c8f6b376b166324992be5431f6d4be131c66`
- Two lifecycle statistics implementations and their two test classes.
- GitHub file-list comparison: not applicable because no GitHub PR
number was provided; the user-specified local `HEAD^..HEAD` range contains
exactly these four files.
- Verification:
- Current-source IDE compilation of all four changed files passed.
- `./mvnw -pl lifecycle -DskipITs -DskipTests=false test`: exit code
`0`; 48 tests passed.
- Spotless, Checkstyle, and Apache RAT passed on the reviewed tree.
- Not Reviewed Scope:
- GitHub Actions and repository merge gates.
- Full multi-JDK and multi-OS build matrix.
- Sharding-assignment, instance-traversal, and etcd cache-lifecycle
follow-up changes, which are outside this commit.
- Release Note / User Docs: not required because the commit corrects
internal lifecycle read consistency without changing public APIs,
configuration, migration requirements, or operational procedures.
--
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]
