[GitHub] [helix] pkuwm commented on issue #642: Fix handleNewSession creating ephemeral node with expired session

[GitBox <gi...@apache.org>]

pkuwm commented on issue #642: Fix handleNewSession creating ephemeral node with expired session
URL: https://github.com/apache/helix/pull/642#issuecomment-570964960
 
 
   > One general comments:
   > Currently, the code for create session aware ephemeral node is looking good.
   > However, we can't simply reply on curren ZKHellixManager code.
   > The reason is that the handleNewSession delivery time is different. For non-shared client, we actually deliver them only when synconnected state change is notified. (This is to get the session_id for ZkClient event queue).
   > 
   > To validate ZKHelixManager working, we should add two different test cases, one for participant role, and the other for controller role. At session expiration and disconnect time, the handleNewSession would be delivered correctly. No missing, no duplication and thread-safe.
   > 
   > Let us add these tests.
   
   Synced up with @kaisun2000.
   
   - For ADMINISTRATOR type using shared zkclient, handleNewSessionAsParticipant or handleNewSessionAsController is not run in handle new session. There is not meaningful things to do in handle new session. So it is fine for shared zkclient.
   - For the other types using dedicated zkclient, existing and newly added tests already cover the cases. 

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org