sandynz opened a new issue, #2528:
URL: https://github.com/apache/shardingsphere-elasticjob/issues/2528

   ## Bug Report
   
   ### Expected behavior
   
   After `JobOperateAPI.disable()` or `JobOperateAPI.enable()` completes, 
lifecycle statistics should reflect the server status stored in ZooKeeper.
   
   ### Actual behavior
   
   When a running job has registered a `CuratorCache`, lifecycle statistics can 
read the previous server status from the local cache after ZooKeeper already 
contains the new value.
   
   For example, after a server node has changed from `ENABLED` to `DISABLED`:
   
   - `JobStatisticsAPI.getJobsBriefInfo(ip)` can still return `OK`;
   - `JobStatisticsAPI.getJobBriefInfo(jobName)` can fail to report that all 
servers are disabled;
   - `ServerStatisticsAPI.getAllServersBriefInfo()` can undercount 
`disabledJobsNum`.
   
   This is related to #2526, but it affects mutable runtime server status 
rather than job configuration.
   
   ### Affected code
   
   `JobStatisticsAPIImpl` reads server status with cached 
`CoordinatorRegistryCenter.get()` in:
   
   - `isAllDisabled(JobNodePath)`;
   - `getJobStatusByJobNameAndIp(String, String)`.
   
   `ServerStatisticsAPIImpl` also uses cached `get()` while calculating 
`disabledJobsNum` in:
   
   - `getAllServersBriefInfo()`.
   
   `JobOperateAPIImpl.disable()` and `enable()` persist the new value to 
ZooKeeper. `ZookeeperRegistryCenter.persist()` updates an existing node through 
`update()`, while the observer's `CuratorCache` is refreshed asynchronously.
   
   ### Reproduction
   
   The reproduction uses two `ZookeeperRegistryCenter` instances in the same 
namespace:
   
   1. The observer creates `/server_status_job/servers/127.0.0.1` with 
`ENABLED`.
   2. The observer registers a cache for `/server_status_job`.
   3. The harness captures the real cached `ChildData` and freezes only that 
server path through a test adapter. Other cache behavior is delegated to the 
real `CuratorCache`.
   4. The writer calls `new JobOperateAPIImpl(writer).disable(jobName, 
serverIp)`.
   5. The observer compares `get()`, `getDirectly()`, `JobStatisticsAPIImpl`, 
and `ServerStatisticsAPIImpl`.
   6. The real cache is restored and allowed to converge.
   
   The frozen cache seam makes the otherwise asynchronous propagation window 
deterministic. The underlying real Curator cache window was independently 
reproduced for #2526.
   
   Observed output on the current `master` branch:
   
   ```text
   cached=ENABLED direct=DISABLED lifecycleStatus=OK
   cached.version=0 cached.mzxid=2877385 direct.version=1 direct.mzxid=2877386
   
   cached=ENABLED direct=DISABLED disabledJobsNum=0
   cached.version=0 cached.mzxid=2877385 direct.version=1 direct.mzxid=2877386
   
   CONVERGED cached=DISABLED
   RESULT jobStatisticsStaleRead=true serverStatisticsStaleRead=true
   ```
   
   The newer ZooKeeper `version` and `mzxid` show that the authoritative value 
is already `DISABLED`. Both lifecycle results still match the cached `ENABLED` 
value.
   
   ### Environment
   
   - ElasticJob branch: `master`
   - ElasticJob commit: `0694c8f6b376b166324992be5431f6d4be131c66`
   - ElasticJob version: `3.0.6-SNAPSHOT`
   - Curator client: `5.9.0`
   - ZooKeeper client: `3.9.5`
   - ZooKeeper server: `3.9.3`
   - JDK: `21.0.10`
   
   ### Root cause
   
   `ZookeeperRegistryCenter.get()` returns data from a registered 
`CuratorCache` when the target node is present in that cache. ZooKeeper writes 
and Curator cache updates are separate asynchronous paths.
   
   The three lifecycle statistics reads above are management-facing queries for 
mutable server status, but they use cached `get()` instead of an authoritative 
read. They can therefore expose the cache propagation window to callers.
   
   ### Proposed change
   
   Use `CoordinatorRegistryCenter.getDirectly()` for server status reads in:
   
   - `JobStatisticsAPIImpl.isAllDisabled()`;
   - `JobStatisticsAPIImpl.getJobStatusByJobNameAndIp()`;
   - `ServerStatisticsAPIImpl.getAllServersBriefInfo()`.
   
   Please keep the following behavior unchanged:
   
   - `ZookeeperRegistryCenter.get()` and its cache semantics;
   - per-job cache registration;
   - scheduling-kernel cached reads;
   - provider SPI signatures.
   
   Add focused tests that verify these lifecycle server-status paths use 
authoritative reads. A provider-level regression should preserve the 
stale-cache snapshot while asserting that lifecycle results follow the direct 
value.
   
   ### Scope boundary
   
   This issue covers only server status values under 
`/<jobName>/servers/<serverIp>`.
   
   Sharding assignment consistency and instance traversal races have different 
multi-node and node-deletion semantics and should be handled separately.


-- 
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]

Reply via email to