Patch for ZOOKEEPER-2184 feedback

Discussion:

powell molleti

2017-03-30 04:25:18 UTC

Hi,

I was wondering if anyone has cycles to look at the PR I have for ZOOKEEPER-2184: Resolve address only on demand ( https://github.com/apache/zookeeper/pull/199 ).

Let me know if I am heading in the wrong direction any pointers will help me to use these changes or drop them from a different PR I have.

thanks
Powell.

Michael Han

2017-03-30 22:54:27 UTC

Permalink

HI Powell,

Have you looked at the existing PR (
https://github.com/apache/zookeeper/pull/150) for ZOOKEEPER-2184? I think
that's what community is working on, and it's close to get merged, so
probably worth to adjust your work on top of that issue?

Post by powell molleti
Hi,
I was wondering if anyone has cycles to look at the PR I have for
ZOOKEEPER-2184: Resolve address only on demand (
https://github.com/apache/zookeeper/pull/199 ).
Let me know if I am heading in the wrong direction any pointers will help
me to use these changes or drop them from a different PR I have.
thanks
Powell.

--
Cheers
Michael.

powell molleti

2017-03-31 06:23:13 UTC

Permalink

Hi Michael,

I did look at it and I can attempt to rebase to it that should not be a problem but that again the changes could undo most of it.
I am pointing to the comment:
https://issues.apache.org/jira/browse/ZOOKEEPER-2184?focusedCommentId=15873730&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15873730

Which quotes
"The ideal situation for the problematic scenario is that we resolve the host name every time we try to connect to a server, but that would be a fairly fundamental change to how we resolve addresses in ZooKeeper."

We can move this conversation to the Jira. I posted my changes after reading that comment since I felt like these changes could address this issue or at-least is headed in that direction.

General idea is that we could let hostname resolution happen right before socket connect call. If a customer never provided hostname perhaps it is incorrect to perform reverse name lookup for it. If a customer did provided a hostname then perhaps it is incorrect to perform a reverse name lookup using the address we resolved(for the given name) later on and use this instead of the given hostname.

Please advise.

thanks
Powell.
On Thursday, March 30, 2017 3:55 PM, Michael Han <***@cloudera.com> wrote:

HI Powell,

Have you looked at the existing PR (
https://github.com/apache/zookeeper/pull/150) for ZOOKEEPER-2184? I think
that's what community is working on, and it's close to get merged, so
probably worth to adjust your work on top of that issue?

--
Cheers
Michael.

Flavio Junqueira

2017-03-31 09:56:41 UTC

Permalink

I'd love to see ZK-2184 fixed. If you have come up with a PR, Powell, I'd be happy to have a look and see if we can converge to a common set of changes.

Michael is right that there is already a PR there, so we would eventually need to decide whether to make changes to it, drop or what.

-Flavio

Post by powell molleti
Hi Michael,
I did look at it and I can attempt to rebase to it that should not be a problem but that again the changes could undo most of it.
https://issues.apache.org/jira/browse/ZOOKEEPER-2184?focusedCommentId=15873730&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15873730
Which quotes
"The ideal situation for the problematic scenario is that we resolve the host name every time we try to connect to a server, but that would be a fairly fundamental change to how we resolve addresses in ZooKeeper."
We can move this conversation to the Jira. I posted my changes after reading that comment since I felt like these changes could address this issue or at-least is headed in that direction.
General idea is that we could let hostname resolution happen right before socket connect call. If a customer never provided hostname perhaps it is incorrect to perform reverse name lookup for it. If a customer did provided a hostname then perhaps it is incorrect to perform a reverse name lookup using the address we resolved(for the given name) later on and use this instead of the given hostname.
Please advise.
thanks
Powell.
HI Powell,
Have you looked at the existing PR (
https://github.com/apache/zookeeper/pull/150) for ZOOKEEPER-2184? I think
that's what community is working on, and it's close to get merged, so
probably worth to adjust your work on top of that issue?

--
Cheers
Michael.

powell molleti

2017-03-31 14:45:57 UTC

Permalink

Hi Flavio,

Here are the changes: https://github.com/apache/zookeeper/pull/199

Let me know if this is headed in the right direction due to the fact that I had to modify many files to accommodate changes to StaticHostProvider and its API.

We can move the discussion to the JIRA if required.
thanks
Powell.

On Friday, March 31, 2017 2:56 AM, Flavio Junqueira <***@apache.org> wrote:

I'd love to see ZK-2184 fixed. If you have come up with a PR, Powell, I'd be happy to have a look and see if we can converge to a common set of changes.

Michael is right that there is already a PR there, so we would eventually need to decide whether to make changes to it, drop or what.

-Flavio

--
Cheers
Michael.