pkuwm commented on pull request #1066:
URL: https://github.com/apache/helix/pull/1066#issuecomment-668990390
> A high level question. The following approach basically encode the session
info to the data of ZkClient? Last time in the design discussion I guess people
raised the concern that this mean ZkClient need to understand the data format.
Normally lower level do not understand higher level data.
>
> Though you can also think of session id in the data as meta data, just
like TCP has out of line control data.
>
> Either way is fine to me. Just want to make sure @pkuwm and @lei-xia agree
with this approach.
>
> ```
> private String parseExpectedSessionId(Object data) {
> if (!(data instanceof SessionAwareZkWriteData)) {
> return null;
> }
> return ((SessionAwareZkWriteData) data).getExpectedSessionId();
> ```
We add a new interface SessionAwareZkWriteData to make it generic:
representing data is session aware. In future, we may implement this interface
to have a public one like SessionAwareZNRecord . With this new interface,
ZkClient doesn’t need to change: treating SessionAwareZkWriteData as generic
one.
How about this? @lei-xia @jiajunwang @kaisun2000
Considering the new interface would tie session to zk, it is not generic
enough. Eg. if we change the storage to mysql, it doesn’t have session.
How about this, splitting the PR into 2: 1st one (this one) doesn’t have new
interface, 2nd one will have the new interface. If we don’t agree on adding new
interface, we just discard the 2nd PR, being tracked in
https://github.com/apache/helix/issues/1219.
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]