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

Feature request: Adding refresh rate to GIT repositories #749

Closed
wants to merge 1 commit into from

Conversation

@karl-ravn
Copy link
Contributor

karl-ravn commented Jul 21, 2017

To avoid overload on GIT backend servers, I have added a RefreshRate setting to the git repositories. This is a naive implementation, which in case of an error will wait another refreshRate until checking again. With this, the configuration server can handle much more load in case of the fact that there are multiple servers asking for updated configuration (or, as in our case, very fine grained configuration from hundreds of files).

The code is completely passive unless activated by configuring the git backend with a "refreshRate" property (number of seconds as value). It is placed next to the "timeout" property, so the same value will be used, just as the timeout.

@pivotal-issuemaster

This comment has been minimized.

Copy link

pivotal-issuemaster commented Jul 21, 2017

@karl-ravn Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster

This comment has been minimized.

Copy link

pivotal-issuemaster commented Jul 21, 2017

@karl-ravn Thank you for signing the Contributor License Agreement!

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 21, 2017

Codecov Report

Merging #749 into master will increase coverage by 0.08%.
The diff coverage is 92.3%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #749      +/-   ##
==========================================
+ Coverage   74.68%   74.76%   +0.08%     
==========================================
  Files          82       82              
  Lines        2698     2711      +13     
  Branches      382      384       +2     
==========================================
+ Hits         2015     2027      +12     
  Misses        522      522              
- Partials      161      162       +1
Impacted Files Coverage Δ
.../server/environment/JGitEnvironmentRepository.java 82.35% <100%> (+0.85%) ⬆️
...environment/MultipleJGitEnvironmentRepository.java 77.77% <50%> (-0.37%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 541fbff...bbad51d. Read the comment docs.

@lizell

This comment has been minimized.

Copy link

lizell commented Sep 13, 2017

This would be super useful, we are having constant problems with a server that is being hit quite frequently, due to a lot of different profiles and settings.

@karl-ravn

This comment has been minimized.

Copy link
Contributor Author

karl-ravn commented Sep 14, 2017

Wouldn't this PR solve the purpose for this old issue? #566

@p-zalejko

This comment has been minimized.

Copy link

p-zalejko commented Oct 11, 2017

any chance to push this topic further?

@karl-ravn

This comment has been minimized.

Copy link
Contributor Author

karl-ravn commented Oct 11, 2017

The interest seems to be a little bit low. We have in the mean-time developed a different solution where we jack into the Spring ApplicationEnvironmentPreparedEvent and performs a double-clone of the repository and rewrites the config server repo to be a local repo instead. The double-clone is to make sure that the repo that Config Server is looking at is only doing refresh against the other local repo on each invocation. The repository that config server is not checking out itself, is indeed a remote repo and refreshed every X seconds.

We had a drop in response times from ~250ms on a small repo to ~80ms with the other fix. It is more complex code however, and the porting is not equally easy.

@ogrodnek

This comment has been minimized.

Copy link
Contributor

ogrodnek commented Mar 23, 2018

I'd love to see this merged. @spencergibb what else needs to be done? If there's anything I can do to push this forward, I'm happy to, just let me know.

thanks!

@spring-cloud spring-cloud deleted a comment from karl-ravn Mar 23, 2018
@spencergibb spencergibb added this to the 2.0.0.RC1 milestone Apr 12, 2018
@spencergibb

This comment has been minimized.

Copy link
Member

spencergibb commented Apr 12, 2018

Merged via 17a2cad

@spencergibb

This comment has been minimized.

Copy link
Member

spencergibb commented Jul 27, 2018

It is in version 2.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.