sandynz commented on PR #2532:
URL:
https://github.com/apache/shardingsphere-elasticjob/pull/2532#issuecomment-5112146459
### Summary
**Review Result: Mergeable**
**Reason:** The latest commit fixes the cache-staleness source at both
lifecycle assignment consumers without changing registry-center, kernel, API,
or SPI contracts. This result is code-scope only; CI was not reviewed
by request.
### Evidence
- `JobStatisticsAPIImpl#isHasShardingFlag()` now reads
`/<job>/sharding/<item>/instance` through `getDirectly()`, so job status
follows the authoritative assignment rather than a potentially stale provider
cache.
- `ShardingStatisticsAPIImpl#getShardingInfo()` applies the same
direct-read policy before resolving instance ID and server IP.
- The change matches the provider contract:
- `ZookeeperRegistryCenter#get()` may return a `CuratorCache` value.
- `ZookeeperRegistryCenter#getDirectly()` reads through the Curator
client.
- `EtcdRegistryCenter#getDirectly()` likewise bypasses its local cache.
- Regression tests distinguish cached and authoritative assignment values:
- `JobStatisticsAPIImplTest` verifies that a stale cached assignment
does not incorrectly produce `SHARDING_FLAG`.
- `ShardingStatisticsAPIImplTest` verifies that the current instance ID
and server IP are returned instead of the cached previous assignment.
- No public API, SPI, configuration, dependency, packaging, or
persisted-data format was changed.
- The existing non-atomic relationship between assignment, instance
metadata, and marker nodes remains outside this narrowly defined fix and is not
newly introduced.
### Review Details
- **Review Focus:** Code Correctness Review — CI not reviewed by request.
- **Reviewed Scope:** `lifecycle` production and test changes in commit
`7aa0a05dbdfadaafa79c3352b09b57e20143b966`; local merge-base
`8ad78c68410e96f806ef4b1aa56641d245fd8f9e`. All four changed files were
reviewed. A
GitHub `/pulls/{number}/files` comparison was not applicable because the
target was a local branch and commit rather than a numbered PR.
- **Not Reviewed Scope:** GitHub Actions, live ZooKeeper/etcd integration
execution, and the separate instance-traversal/deletion-race follow-up.
- **Verification:**
- `./mvnw -pl lifecycle -DskipITs -DskipTests=false test` — exit 0; 48
tests passed.
- `./mvnw spotless:check -Pcheck -T1C` — exit 0.
- `./mvnw checkstyle:check -Pcheck -T1C` — exit 0; 0 violations.
- **Release Note / User Docs:** Not required. This is an internal
lifecycle consistency correction with no user-facing configuration, API,
migration, or compatibility change.
--
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]
