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

A gentler auto-update mechanism #6692

Closed
ethomson opened this issue Jul 27, 2016 · 8 comments
Closed

A gentler auto-update mechanism #6692

ethomson opened this issue Jul 27, 2016 · 8 comments

Comments

@ethomson
Copy link

@ethomson ethomson commented Jul 27, 2016

Hello!

I'm @ethomson, a member of GitHub's Git Infrastructure team, which is responsible for ensuring that GitHub's file servers are nice and healthy and serving git repositories to clients efficiently. Part of this work is applying quotas and automatic rate limiting to ensure that no individual repository consumes all of GitHub's resources.

I'm opening this issue because we have been periodically detecting very high load on the syl20bnr/spacemacs repository. When this occurs, we will begin applying quotas to actions on this repository, and automatically delaying git operations to preserve the health of our file servers. As a result, operations like git fetch will be delayed and - when an extraordinarily high number of concurrent requests occur that we could not possibly serve - we will simply begin refusing connections from git clients entirely. This is happening pretty regularly to the syl20bnr/spacemacs repository.

After some brief investigation, I suspect that much of this load is coming from the automatic update mechanism. (I regret that I am not an emacs user, and not super familiar with Emacs Lisp, so I apologize in advance if I'm misinterpreting the update mechanism.)

It looks like spacemacs is doing a git fetch <remote>, followed by a git fetch <remote> <branch>, followed by a git fetch --tags <remote> <branch>. These three individual commands should be unnecessary - the git fetch --tags <remote> <branch> should be enough to get both the tags and the commits that you're interested in.

Reducing this from 3 operations to 1 when checking for updates would reduce the number of concurrent git operations significantly. This would speed up the new version check in all cases, but this would be an especially helpful change when the fetches are slow (for example, when we're intentionally slowing them down 😀 ).

I think that this change alone would be enough to bring the syl20bnr/spacemacs repository back into a place where it's healthy and we are not applying quotas to it. But I would also encourage you to limit the frequency with which you poll -- at present, it looks like you're checking for updates at startup. This seems reasonable for people who keep their emacs running and use a mechanism like emacsclient to open a new buffer, but it looks like I'm seeing the same clients getting delayed (or outright refused) over and over again, which I suspect is people who restart new instances of emacs (and thus check for updates at every startup). If the startup check could also honor some sort of timestamp and also only check for updates every 6 hours that would also help here.

Finally, using a cheaper git operations than doing a git fetch would also be nice. You could query the tags and only do the fetch when you know that you have a new update to download.

Apologies for the wall of text, and thanks for reading. 😀 I think that some minor changes to the automatic update mechanism would reduce concurrency to a place where GitHub is no longer applying quotas, which will be very beneficial for spacemacs users.

@syl20bnr
Copy link
Owner

@syl20bnr syl20bnr commented Jul 27, 2016

Hi @ethomson,

Thank you for helping us to help you :-) The pointers are greatly appreciated. We will update our process to lower the load on your infrastructure.

@ethomson
Copy link
Author

@ethomson ethomson commented Jul 27, 2016

@syl20bnr My pleasure! Please let me know if I can help. 😀

@syl20bnr syl20bnr self-assigned this Jul 31, 2016
syl20bnr added a commit that referenced this issue Aug 4, 2016
Since git commands contacts remotes hosted on GitHub this settings
should be an opt-in.

Fixes #6692
syl20bnr added a commit that referenced this issue Aug 4, 2016
syl20bnr added a commit that referenced this issue Aug 4, 2016
Once per 24 hours by default.
Caveat: if a new version is detected on the current instance of Emacs
and Emacs is restarted before the end of the 24 hours window then
the new version lighter in the modeline disappear; i.e. we don't
remember the last check results.

Fixes #6692
@syl20bnr
Copy link
Owner

@syl20bnr syl20bnr commented Aug 4, 2016

@ethomson I pushed the fixes to the develop branch. They will be merged to master this month when the new release is ready, you may not see any improvement until the fixes are merged. I can check to backport them sooner if you really want me to do it.

The fixes consist of the following changes:

  • remove the unnecessary git commands and keep only git fetch --tags <remote> <branch>
  • change the default value of dotspacemacs-check-for-update from t to nil (i.e. checking for new version at startup is now an opt-in setting)
  • rate limit the version checks at startup to once per day
  • remove the reccurent version check every 6 hours

If we still popup in your monitoring with these changes then it would mean that Spacemacs is more popular than Atom 😸

(or we have a bug.... 🐞 )

@ethomson
Copy link
Author

@ethomson ethomson commented Aug 8, 2016

I can check to backport them sooner if you really want me to do it.

No, there's definitely no need for that. :) If we detect very high load again, we'll rate limit and slow down concurrent git fetches to your repository a little bit. (And it really is just a little bit, it's an uncommon occurrence that we even apply quotas to your repo.)

If you're happy, we're happy. 😀

Thanks again!

@d12frosted
Copy link
Collaborator

@d12frosted d12frosted commented Oct 3, 2016

Fixed with released of Spacemacs v0.200. Let us know if you still have any problems with this issue.

@gerrywastaken
Copy link

@gerrywastaken gerrywastaken commented Mar 31, 2019

@syl20bnr I'm wondering... given you added the rate limiting to fetches, was it necessary to make the dotspacemacs-check-for-update option opt in? Or perhaps you added this change for a different reason?

@syl20bnr
Copy link
Owner

@syl20bnr syl20bnr commented Apr 1, 2019

@gerrywastaken Indeed you are right I made an opt-in for a different reason.
Emacs is a Free software and I want to minimize the possiblity where third-party
services could track you. The check for updates make calls to GitHub hosted
services which could be used to track your activity with Emacs. I know this is
a bit paranoiac but I feel that starting Spacemacs should not be contacting
external services without users consent.

We could makes this more explicit and add a question to the wizard though.

@gerrywastaken
Copy link

@gerrywastaken gerrywastaken commented Apr 9, 2019

@syl20bnr I don't think it's paranoiac, it's thoughtful and I like the decision now that you put it this way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Bugs
To close
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.