Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix cluster uri resolution #5869

Merged
merged 3 commits into from Nov 16, 2015
Merged

Fix cluster uri resolution #5869

merged 3 commits into from Nov 16, 2015

Conversation

spacecowboy
Copy link
Contributor

Reverts part of 5f7bf49

Reason:
When cluster members do not specify ha.server setting (not required),
the slaves are unable to join the cluster in certain cases.

@spacecowboy
Fix cluster uri resolution
577e845 
Reverts part of 5f7bf49

Reason:
When cluster members do not specify `ha.server` setting (not required),
the slaves are unable to join the cluster in certain cases.
@tinwelint tinwelint self-assigned this Nov 10, 2015
@tinwelint
Copy link
Member

Thinking that the behaviour that this PR aims to revert might be useful in some environments (multi-datacenter f.ex.), should we instead of reverting introduce another config option ha.use_host_names or whatever it would be called, where if true would expose the host name instead of the ip address. This setting could be false by default to simplify for the most common case. Sure there would be one more config option, but the amount of config user has to do to just get things going would be less. Not until you're doing serious clustering across data centers you'd have to care about this config and the ha.server setting.

@spacecowboy
Copy link
Contributor Author

@tinwelint We shouldn't introduce new functionality in a patch release. I think it's better to try to fix the default behavior to take these changes into account.

@spacecowboy
Remove ServerUtil
c05c691 
It was scheduled for removal when Neo4j was Java7 only
@spacecowboy
Simplify cluster server address logic
596b65b 
You should NOT resolve the address, NOR should you resolve the
hostname.
@spacecowboy
Copy link
Contributor Author

@tinwelint New suggested behavior pushed

@spacecowboy
Copy link
Contributor Author

Tests show this is insufficient to fix connectivity in all network cases.

Edit: This actually fixes the regression. We have additional problems which will be addressed for 3.0

tinwelint added a commit that referenced this pull request Nov 16, 2015
@tinwelint
Merge pull request #5869 from spacecowboy/2.3-clusterurifix
85aa804 
Fix cluster uri resolution
@tinwelint tinwelint merged commit 85aa804 into neo4j:2.3 Nov 16, 2015
@spacecowboy spacecowboy mentioned this pull request Nov 20, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@tinwelint tinwelint

Labels
2.3 changelog team-kernel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@spacecowboy @tinwelint