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

Rewrite of primary gitolite interface for better resilience and performance. #124

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

kubitron
Copy link

The primary "update_repositories" function is now largely self-correcting against a variety of errors seen in the field. Also added a bunch of error checking on functions called-out to the shell. Among other things, this code attempts to reconnect the gitolite-admin public key when it gets deleted (can happen).

Regular use of 'sys/fetch_changesets' will recorrect any errors in the public key directory and gitolite.conf file. For example, orphan keys that happen to be left behind in the public key directory will be removed, missing keys will be added, etc.

One new piece of functionality is the 'recycle_bin' which is used to store deleted repositories immediately after they are deleted (allowing them to be recovered for up to 'gitRecycleExpireTime' hours after deletion).

This set of patches also includes a number of performance fixes (an improvement over those referenced in pull request #120).

-- Edit 1/10/2012 --
The latest commit (2f1c213) includes a number of changes. First and foremost, the update_repositories() function now handles deletes and repository moves as well as all other changes. Consequently, :RESYNC_ALL will be
able to recover from a variety of problems involving movement of repositories resulting from project parent changes. Note that changes in the parent for a subtree of projects works much better than before.

Second, this commit adds new settings which allow (1) redmine-managed repositories to be focused in a specific subdirectory of the gitolite repository, (2) allow repositories to be named either hierarchically (default) or concentrated in a single directory (flat) independent of project parents, and (3) an extra path parameter which can be added to the http URL for smart http access of repositories.

Third, added the "access" box at the top of the settings page which shows explicitly how settings affect access parameters.

Random improvements include (1) rewrite of routes for smart HTTP to better handle changes in parentage (and avoid need to change routes as project parents change); (2) changes in settings now immediately trigger resulting changes in state (before, the settings cache got in the way); (3) all of the path functions (repository positions, ssh access, http access) have been concentrated into a small number of functions at the top of git_hosting.rb, rather than spread throughout the code; (4) continuing the changes started with the original performance fixes, cleaned up observer behavior triggered by changes in project membership and settings.

-- Edit 1/16/2012 --
One additional commit (c4476f6) will resync the hooks parameters on a fetch_changesets, making sure that the update keys don't get out of sync.

Please grab all 8 commits (since they are interconnected) You need to migrate plugins because there are new plugin settings:

rake db:migrate_plugins

Note that I have incremented the version ID to 0.4.3x in the README.mkd, since this is a pretty major change (but I don't want to mess with Eric's numbering system). The README.mkd file has been updated to reflect changes.

This prevents a rediculous number of calls to update_repositories
caused by the fact that redmine saves to the repository model for
every commit that it examines.  This patch also prevents multiple
calls to update_repositories when adding groups to the membership
of a project.

You need to migrate plugins to take advantage of new indices:

	rake db:migrate_plugins
This patch should remove all remaining need for recursive calls to update_repositories.
Will log such calls as errors in upcoming major code fix...
The primary "update_repositories" function is now largely self-correcting
against a variety of errors seen in the field.  Also added a bunch of
error checking on functions called-out to the shell.  Among other things,
this code attempts to reconnect the gitolite-admin public key when it
gets deleted (can happen).

Regular use of 'sys/fetch_changesets' will recorrect any errors in the
public key directory and gitolite.conf file. For example, orphan keys
that happen to be left behind in the public key directory will be removed,
missing keys will be added, etc.

One new piece of functionality is the 'recycle_bin' which is used to store
deleted repositories immediately after they are deleted (allowing them
to be recovered for up to 'gitRecycleExpireTime' hours after deletion).

You need to migrate plugins because there are new plugin settings:

    rake db:migrate_plugins
This is not a major issue (or even correctness issue), but will cause
complaints of recursion in the log.
@juselius
Copy link

Super, thank you kubitron! I have upgraded our servers, fingers crossed!

@kubitron
Copy link
Author

Let me know how it goes. There is a chance that this patch might make your fetch_changesets() operation (slightly) slower, since the original one was not guaranteed to fix the gitolite.conf file and keydirs for all projects, while this one will. (You will see a notification about "RESYNC_ALL" in the log associated with fetch_changesets()). Thus, I'm really curious about timing. I tried to limit the number of accesses to the file system, so that might actually speed things up.

Also, I seem to remember you saying that you were executing fetch_changesets() every 5 minutes. Depending on your usage pattern, this level of frequency is probably not necessary (since a project-specific fetch_changesets() gets executed whenever anyone looks at the repository link). You could try 10 or 15 minutes....

@kubitron
Copy link
Author

kubitron commented Dec 6, 2011

Ok. Bakerjonas, how's it going? Does this version work for you?

Inquiring minds want to know... Also curious about timing for systems with lots of users.

@juselius
Copy link

juselius commented Dec 6, 2011

Works like a charm. 100+ users, and not a single complaint :) Excellent work, thank you kubitron!

@simonsd
Copy link

simonsd commented Jan 4, 2012

I'd like to give another vote for this pull request.
Using these few extra commits really makes a world of difference,
much more stability and the verbosity in the logs really helps point you in the right direction.

Tested on CentOS 6.2 with:

  • redmine 1.2.1
  • ruby 1.8.7 p352
  • gitolite 2.0.3
  • httpd 2.2.15
  • passenger 3.0.8

change rakefile to install binary policy (redmine_git.pp) instead of
compiling source policy (redmine_git.te) before every
installation. New rake task added to recompile policy if necessary:

    rake selinux:redmine_git_hosting:build_policy
@kubitron
Copy link
Author

kubitron commented Jan 9, 2012

There are two new commits to this branch which finish the cleanup started with the first commits (first one is actually a commit for my selinux branch, but will do nothing if you are not using selinux). Please give them a try.

@kubitron
Copy link
Author

Sorry, all. I accidentally pushed a bug into this last commit. Fixed through history editing (bad idea, I know, trying to keep things simple).

Just kill of last commit and repull.

Would love comments from others on this latest commit. This commit will be the last in this resilience branch (promise), assuming that it works for people.

Any of the old lurkers still out there? bakerjonas? simonsd? Willing to try this new commit?

@juselius
Copy link

I'm here! It's just that I'm currently down in Mexico flying paragliders :) I'll test your patches next week.

@kubitron
Copy link
Author

Great! Far be it from me to interrupt paragliding! Have fun :-).

It is important to remember to migrate_plugin settings, since there
are new settings and one setting has changed a bit (i.e. httpServer
should nolonger include a rails_root path).  Thus, make sure to:

    rake db:migrate_plugins

This commit includes a number of changes.  First and foremost, the
update_repositories() function now handles deletes and repository
moves as well as all other changes.  Consequently, :RESYNC_ALL will be
able to recover from a variety of problems involving movement of
repositories resulting from project parent changes.  Note that changes
in the parent for a subtree of projects works much better than before.

Second, this commit adds new settings which allow (1) redmine-managed
repositories to be focused in a specific subdirectory of the gitolite
repository, (2) allow repositories to be named either hierarchically
(default) or concentrated in a single directory (flat) independent of
project parents, and (3) an extra path parameter which can be added to
the http URL for smart http access of repositories.

Third, added the "access" box at the top of the settings page which
shows explicitly how settings affect access parameters.

Random improvements include (1) rewrite of routes for smart HTTP to
better handle changes in parentage (and avoid need to change routes as
project parents change); (2) changes in settings now immediately
trigger resulting changes in state (before, the settings cache got in
the way); (3) all of the path functions (repository positions, ssh
access, http access) have been concentrated into a small number of
functions at the top of git_hosting.rb, rather than spread throughout
the code; (4) continuing the changes started with the original
performance fixes, cleaned up observer behavior triggered by changes
in project membership and settings.
@juselius
Copy link

Hi kubitron! I just pulled from your repo, and upgraded both redmine and the gitolite plugin. Everything seems to work, so far. I see that Eric has incorporated some of your work (but not all?), and I'm not sure if you have pulled all his changes into your version. The situation is a bit confusing.

@kubitron
Copy link
Author

Hello. I haven't pulled anything from him since the original branch. Did he just make the changes?

(Actually, his master branch seems the same....?)

@kubitron
Copy link
Author

BTW -- the trunk for Redmine is currently broken, even with my patches. Not sure what you pulled...(might want to stick with official releases, such as 1.3-stable).

The development branch has a half-working version of a multiple repo/project solution which is going to complicate things.

@simonsd
Copy link

simonsd commented Jan 19, 2012

first off: thanks for the hard work so far.
second: judging from your comments on http://www.redmine.org/issues/779 ,
I'm assuming you plan on updating the plugin to be compatible with the multiple repos stuff?

lastly: I'm having some issues with the cloning of the gitolite_admin repo.
I understand that the 'gitolite server domain' setting is used for this.
However this is also used for the links provided on the project pages.

The problem lies with the fact that we use an alternate port for accessing the repo's,
which that setting does not understand.
Is it possible to allow for this to be specified, or to create a separate setting for 'internal' use?

@kubitron
Copy link
Author

Hello. No problem on the work -- I needed this to be stable for a couple of projects of mine.

As for continuing updates, this may be a bit problematic: Eric doesn't seem to be willing to do merges, but rather is reimplementing parts (as far as I can tell). Ack! As a result, my branch is going to rapidly diverge -- and I don't have time to back port everything.

With respect to the multiple repo/project feature, I am still trying to figure out what the intended semantics will be (and am interacting with the redmine developers). Since I like to keep up-to-date with the latest releases, I will probably take a stab at dealing with the feature set. Right now, it is broken and under rapid development. I ended up switching back to the 1.3-stable branch off the trunk.

Anyway -- as for your issue, I'm not quite sure I understand. Are you using a different ssh port for gitolite? If so, why wouldn't the same (different) port be used for both gitolite-admin access and ssh access to repositories? The gitolite server setting (gitServer) could easily have a ":port" syntax which would be used for an alternate ssh access (it doesn't at the moment). It would then also show up on the gitURL display on the repo. Is this what you need?

@kubitron
Copy link
Author

p.s. Just trying to understand your setup. It might be easy to upgrade things to support a non-standard gitolite port...

@simonsd
Copy link

simonsd commented Jan 20, 2012

Yeah sorry, i realize my description was rather badly phrased.
I'll give it another try :)

I'm using gitolite on port 22 as normal, but it's not directly available from the internet.
The machine hosting redmine and gitolite is a virtual machine.
Traffic is first being routed through the physical machine, using NAT to forward all traffic from port 2222 to port 22 on the redmine instance.

In this case I would have to specify a 'user@hostname:port' string as the gitServer setting.
However that won't work due to the fact that the rest of the string is hardcoded.
In this case you would end up trying to clone the gitolite-admin repo with a clone url like: 'user@hostname:port:project/repo.git'.

I managed to get this working by hardcoding the clone url's on lines 376 and 395 in lib/git_hosting.rb from this:

shell %[env GIT_SSH=#{gitolite_ssh()} git clone #{git_user}@#{Setting.plugin_redmine_git_hosting['gitServer']}:gitolite-admin.git #{repo_dir}]

to this:

shell %[env GIT_SSH=#{gitolite_ssh()} git clone git+ssh://#{git_user}@#{Setting.plugin_redmine_git_hosting['gitServer']}/gitolite-admin.git #{repo_dir}]

Which then ends up as 'git+ssh://user@hostname:port/project/repo.git'.
Although this fixes the problems with cloning the gitolite-admin repo, it doesn't yet make the clone url's appear correct since those are put together elsewhere.
To get these right I adjusted line 7 in assets/javascripts/git_url_display.js from this:

urls["git_url_ssh"]  = [guGitUser + "@" + guGitServer + ":" + guProjectName, guUserIsCommitter]

to this:

urls["git_url_ssh"]  = ["git+ssh://" + guGitUser + "@" + guGitServer + "/" + guProjectName, guUserIsCommitter]

I guess this could/should be done automatically, based on the presence/absence of a port in the gitServer param,
or with a separate port param if that would be easier.
All that's really needed is a line that checks wether an alternate port is being used, and sets the appropriate vars accordingly.

The reason I was thinking about a separate 'internal' url for cloning the gitolite-admin repo is that I was having some issues with the iptables NAT config.
These issues prevented me from using the external ip to clone the repo on the machine, but I have since resolved said issues.

As for Eric not merging the commits and making more changes, I'm also pushing this into production at my company.
Hence I will continue using your branch, unless Eric comes up with more stability and the same features as you've implemented already.

@kubitron
Copy link
Author

@bakerjonas: Looks like Eric only grabbed the performance enhancements (my performance branch), not the resilience fixes (this branch). Thus, the new branch likely still has the problems of the old branch that would get in an inconsistent state.

Incidentally, have you noticed any problems with the latest commits? I know that you have a lot of users, so you would make a good test case.

@kubitron
Copy link
Author

@simonsd. Ok. I'm assuming by what you said that you are using NAT magic on the Redmine box to allow the same ip:port to be used for local (admin) and remote cloning?

Why don't you post an "issue" over on my Fork asking for the ability to have a :port in the gitServer parameter. This request seems reasonable, since my updated README mentions that as a valid syntax for the httpServer already (and goes to the trouble of pointing out that you shouldn't put a port in the gitServer parameter).

WRT the original topic, are you successfully using the latest version of my "resilience" commits (or pulled off my head of master recently)? Someone mentioned a problem, but they seem to have other problems at the moment, so not sure what's up...

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.

3 participants