ISPN-1826 Make sure address cache is updated before comparing view ids #923
Conversation
|
Oh, forgot to say, 5.1.x too: t_1826_5 |
ghost
assigned 
| // Could this be a lazySet? We want the value to eventually be set, | ||
| // clients could wait a little bit for the view id to be updated | ||
| // on the server side... | ||
| viewId.set(localViewId) |
There was a problem hiding this comment.
What happens if you have more than one node joining or leaving at the same time?
I think it would be safer if CrashedMemberDetectorListener would update the view id after updating the cache.
There was a problem hiding this comment.
Also, if it's ok for clients to see the new view id a little later, couldn't the viewId parameter of writeHeader and getTopologyResponse be an int instead of AtomicInteger? Seeing AtomicInteger makes me think that those methods want to update its value, but they only need to read it.
There was a problem hiding this comment.
The view is already updated after updating the cache, that's why isPre must be false :)
There was a problem hiding this comment.
Hmmm, I see what you mean. However, what you suggest requires updating the view id in two places. In CrashedMemberDetectorListener and the startup. This way is limited to only beeing updated once we know the cache has been updated.
This avoids a race condition where a view id might be updated before the address cache, so clients could end up receiving views that do not contain the address updates.
|
@danberindei I've updated the pull request, can you check? |
https://issues.jboss.org/browse/ISPN-1826
This avoids a race condition where a view id might be updated before the address cache, so clients could end up receiving views that do not contain the address updates.