Fix stale server status in lifecycle statistics by using getDirectly() instead of get() - #2529
Open
waterWang wants to merge 1 commit into
Open
Fix stale server status in lifecycle statistics by using getDirectly() instead of get()#2529waterWang wants to merge 1 commit into
waterWang wants to merge 1 commit into
Conversation
…) instead of get() Switched from regCenter.get() to regCenter.getDirectly() for server status reads in JobStatisticsAPIImpl and ServerStatisticsAPIImpl. The get() method returns data from CuratorCache, which can return stale values during the async cache propagation window after ZooKeeper writes. getDirectly() always reads the authoritative value from ZooKeeper. Affected methods: - JobStatisticsAPIImpl.isAllDisabled() - JobStatisticsAPIImpl.getJobStatusByJobNameAndIp() - ServerStatisticsAPIImpl.getAllServersBriefInfo() Fixes apache#2528
linghengqian
left a comment
linghengqian
left a comment
Member
There was a problem hiding this comment.
You obviously need to resolve the merge conflict.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2528
Problem
JobStatisticsAPIImpl.isAllDisabled(),JobStatisticsAPIImpl.getJobStatusByJobNameAndIp(), andServerStatisticsAPIImpl.getAllServersBriefInfo()read server status viaregCenter.get(), which returns data from CuratorCache. AfterJobOperateAPI.disable()orJobOperateAPI.enable()writes the new value to ZooKeeper, the cache refresh is asynchronous — there is a window where the cached value is still the old one.For example, after a server node changes from
ENABLEDtoDISABLED:JobStatisticsAPI.getJobsBriefInfo(ip)can still returnOKJobStatisticsAPI.getJobBriefInfo(jobName)can fail to report that all servers are disabledServerStatisticsAPI.getAllServersBriefInfo()can undercountdisabledJobsNumFix
Use
regCenter.getDirectly()instead ofregCenter.get()for server status reads in the three methods above.getDirectly()always reads the authoritative value from ZooKeeper, bypassing the local cache.Scope
Only server status values under
/<jobName>/servers/<serverIp>. Sharding assignment consistency and instance traversal races are not affected.Testing
Updated existing unit tests to mock
getDirectly()instead ofget()for server status paths — the tests verify the same behavior (DISABLED → DISABLED, mixed → OK, per-IP → DISABLED) against the authoritative read path.