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

Config Server pulling repo in each request #566

Open
fjvieira opened this Issue Nov 24, 2016 · 7 comments

Comments

Projects
None yet
7 participants
@fjvieira

fjvieira commented Nov 24, 2016

In the Config Server, every time it tries to fullfil a GET request for a GIT repository, it calls refresh method of JGitEnvironmentRepository, that results in a PULL to GIT repo, what takes a lot of time and can generate issues in case of many concurrent requests. Maybe it should be better to have a timeout threshold and a scheduled task to do it.

@rlynch

This comment has been minimized.

Contributor

rlynch commented Nov 24, 2016

At my company we have a similar problem and are working on a PR which will use messaging to proactively update the locally cloned repos on a git update and almost entirely eliminate the need for a pull request during a confit get request. We want to do some internal testing and hardening before submitting it here but look for it soon. We are not committers so there is no guarantee that it will ever be accepted in SCCS so don't run your business counting on this but we will have something for the community to review in the near future.

@spencergibb

This comment has been minimized.

Member

spencergibb commented Nov 24, 2016

@rlynch how different is that from the monitor functionality?

@fjvieira

This comment has been minimized.

fjvieira commented Nov 24, 2016

@spencergibb The monitor functionality is nice. I adapted the monitor solution to use SQS/SNS instead of Cloud Bus and it works very well. My point is that should be a way of overpassing what I described. Nowadays the average response time is more than 1 second. And considering that health check of each Config Client do a GET to print the list of resources, it is an eternity. I disabled this health check in my clients and the response time of /health now is less than 20ms.

@rlynch

This comment has been minimized.

Contributor

rlynch commented Nov 24, 2016

Not much, only difference is it updates the configuration servers instead of the clients. Then we can "mostly" remove the call to git during the findOne method. This also allows us to reduce the locking on the findOne method which really hindered scalability for us. It's coded up but still needs to be tested so it only works in unit test land at this point.

@marcellodesales

This comment has been minimized.

Contributor

marcellodesales commented Nov 29, 2016

@fjvieira that's definitely a great finding for sure! We were asked to investigate the health check as well, but we considered not using it... I've been working with @rlynch and his patch should help help with your requirement 👍

@kuipercm

This comment has been minimized.

kuipercm commented Dec 2, 2016

@rlynch (How) Can we help? This is a feature that would be most welcome in our environment as well. The load and response time caused by the git pull is too much at the moment, so avoiding git pulls would be very helpful.

@asarkar

This comment has been minimized.

Contributor

asarkar commented Jan 25, 2017

I've a different thought about this. If we could add caching to config server then the constant Git pull could be reduced. This would have been real easy to do using Spring AOP if JGitEnvironmentRepository.refresh wasn't a private method. AFAIK, Spring AOP can't proxy private methods. Perhaps a small PR can change that.
Of course, using the monitor eliminates the pull totally but at the cost of an added dependency on a queue. And I don't think the monitor works for multiple instances of config server without additional code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment