New API to create an ephemeral node that is session aware

[huizhilu]

• Creates an ephemeral node that is session aware.
• If the ephemeral node is created successfully, returns true.
• If session is expired, returns false.
• If connection is timed out or interrupted, exception is thrown.

I will complete the descriptions once the code itself is agreed. We will focus on the code logic in this draft.

[narendly]

Please explain what "session aware" means here as well?

[huizhilu]

Yes, will do.

[kaisun2000]

"Creates an ephemeral node that is session aware.
If the ephemeral node is created successfully, returns true.
If session is expired, returns false.
If connection is timed out or interrupted, exception is thrown."

The description is not very accurate. The gist is that given a session id, we need to make sure the created ephemeral node must be in this session. Otherwise, the creation should fail.
ZkClient has a auto-reconnect feature. This feature makes this creating ephemeral node in this session operation not that easy.
We need handle session expired, disconnected state change. In additional close().

So all in all the following semantic should be true.

"
given a session ID, the api should return true it can create an ephemeral node in this session.
If met irrecoverable condition, the API should return false.
If met recoverable condition, the API should retry and create the ephemeral node if possible.
"

Here, irrecoverable condition can be: session expired, new session established, zkclient closed, interrrupted, keeper exceptions such as invalid parameters etc.

Recoverable condition can be: session disconnect. (Note, session can still be reconnected with the same session id potentially later.)

[huizhilu]

Right, thanks for the meaningful description. This is what I am planning to complete. Once the code itself is agreed by us, I will complete the description.

[huizhilu]

If we get SessionExpiredException here, the exception would be caught in retryUntilConnected(), and waitForRetry() will be run. And in the next connection in retryUntilConnected(), we get that getSessionId() != sessionId and so return false. It means, we don't return false right after catching SessionExpiredException, but waitForRetry and return false in the next connection. So false would be returned a bit delay.

[huizhilu]

@lei-xia's comment:
lei-xia 2 minutes ago Many of the code in this method is duplicated with the one in create() (line 522). Another option is to extend that method to take a sessionID (also wrap the content of that method as a new private method to avoid expose it to be public API)?

My reply: I tried to reduce the duplicate code. There are 2 things to consider:

1. Because the input data is an object, we need to serialize it. If so, we need to change the api parameters like:
   create(final String path, byte[] data, final List acl, final CreateMode mode, Callable callable);
   And we will need to change create() implementation, whether or not we want to do?
2. Object data needs to be serialized and passed to the final create().
   I will show you the change for the above idea and see if we could accept it.

[huizhilu]

Lei's comments: huizhilu@f6feb71

[lei-xia]

Can we have only 1 PR and keep all comments in one place. It is a bit confusing here.

[huizhilu]

@lei-xia 's comment:
Give a more clear description what does this extra sessionId in parameters mean, such as, this method allows you to specify an expected sessionId, if the current sessionId of underlying ZK connection does not match the given sessionId, the creation will fail ....

[huizhilu]

Lei @lei-xia
This is should be the expected session Id of the ZK connection, not necessarily the sessionID of current ZK Connection, rephrase it to make it less confusing. And also make it clear that if the sessionID of current ZK connection does not match the given (expected) sessionId, the operation will fail.

[junkaixue]

If we think this is fundamental change, why not we have another implementation of retryUntilConnect instead of moving code to createEmphemeNode?

[huizhilu]

@dasahcc In this PR, I don’t move the code retryUntilConnect to createEmphemeNode, instead I reuse retryUntilConnect to minimize the changes.

…

On Fri, Nov 22, 2019 at 10:36 AM Junkai Xue ***@***.***> wrote: ***@***.**** commented on this pull request. If we think this is fundamental change, why not we have another implementation of retryUntilConnect instead of moving code to createEmphemeNode? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#630?email_source=notifications&email_token=ABHSRCMHT7LCUMVP62RKMBLQVAREPA5CNFSM4JQT74J2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCMWUGXA#pullrequestreview-321733468>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABHSRCOMFIP4FXNYBB5WOE3QVAREPANCNFSM4JQT74JQ> .