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.

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

resolve endpoint zone from metadata #1287

Merged
merged 2 commits into from
May 27, 2020
Merged

resolve endpoint zone from metadata #1287

merged 2 commits into from
May 27, 2020

Conversation

MI-cool
Copy link
Contributor

@MI-cool MI-cool commented Apr 26, 2020

endpoint zone can be get from metadata

@troshko111
Copy link
Contributor

The change LGTM, but can you elaborate on the bug and add some tests showing how it fixes it?

@MI-cool
Copy link
Contributor Author

MI-cool commented May 11, 2020

Sorry.
My project was build with spring-cloud-starter-netflix-eureka-client.
I use the following configuration to make the client dynamically aware of the server nodes.

eureka.client.transport.bootstrapResolverStrategy=composite
eureka.client.transport.readClusterVip=eureka-read-server
eureka.client.transport.writeClusterVip=eureka-write-server

During the test, it was found that the client lost the server zone information. Finally found that the more reasonable way to modify is to add a few lines of code submitted.

@MI-cool
Copy link
Contributor Author

MI-cool commented May 11, 2020

I will try to add some tests.

@MI-cool
Copy link
Contributor Author

MI-cool commented May 25, 2020

Test was done.

@troshko111
Copy link
Contributor

troshko111 commented May 26, 2020

Sorry for the delay, this fell off my attention, I promise to have a short turnaround now.
I looked around the code and seems like this is Aws-specific path (see the return type) so it's expected to not to have the zone info on MyDatacenterInfo, there's no contract saying there should be any zone there in the meta. What issues it this causing?

@MI-cool
Copy link
Contributor Author

MI-cool commented May 27, 2020

My eureka server was build with spring-cloud-starter-netflix-eureka-server. On server side, the framework build instances use dataCenterInfo type of MyDataCenterInfo as default.
My eureka client was build with spring-cloud-starter-netflix-eureka-client. I use eureka.client.transport.bootstrapResolverStrategy=composite to make the client dynamically aware of the server nodes. Here comes the problem, since newBootstrapResolver lost zone of server instances, eureka clients can't be zone affinity to server instances.

@troshko111 troshko111 merged commit 866ea50 into Netflix:master May 27, 2020
@troshko111
Copy link
Contributor

Yeah I think this is reasonable, the current AWS/non-AWS separation is weak so I don't see a better place to do this tbh. Thanks and sorry for the delay.

@MI-cool
Copy link
Contributor Author

MI-cool commented May 28, 2020

No worries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants