Remove satellites

• remove satellite classes
• make web editor work without satellites
• make compare work without satellites
• make merge work without satellites
• execute pre-receive hook when commits done via web editor or merge in UI !1154 (merged)
• execute post-receive hook when commits done via web editor or merge in UI
• Check #2252 (moved) - should be fixed by !1154 (merged)

Previously reverted by !1136 (merged)

Based on !981 (merged)

cc @sytses @JobV @DouweM @vsizov @rspeicher

---

P.S. After this merged - check effect on EE features

Added 1 commit:

• d25c56cf - Update CHANGELOG

As soon as 7-14-stable is created I am going to merge it

link to #1974 (closed)

mentioned in issue #2247 (closed)

mentioned in issue #1974 (closed)

mentioned in issue #1974 (closed)

@jacobvosmaer do you think its wise to remove satellite directory in migration or better leave it for administrator to do manually?

@dzaporozhets migration in 8.1?

@sytses as an option. Lets leave it for 8.1 then

Cleanup scheduled for 8.1 - #2186 (closed)

Added 23 commits:

• d25c56cf...cb6ad67f - 22 commits from branch master
• 69c193e7 - Merge branch 'master' into remove-satellites

Removing the satellites will be relatively slow, I think it is better to put it in a rake task. If we put it in a migration, and some other migration in the release needs downtime (which is likely), then you end up with long downtime.

Removing the satellites will be relatively slow, I think it is better to put it in a rake task. If we put it in a migration, and some other migration in the release needs downtime (which is likely), then you end up with long downtime.

I agree. It's not essential that the satellite directories be removed, right? It's just extra disk space? In that case I think a manual cleanup task is acceptable.

@jacobvosmaer @rspeicher thank you. it makes sense

I think you should remove satellite directories somehow as part of upgrade otherwise you create more work for admin. I agree you don't want it to prolong downtime however. Any way it can bring gitlab back online and then remove satellites?

Well, you could create a RemoveAllSatellitesWorker, and then inside a migration tell sidekiq to start running it (maybe with a 1-hour delay).

Maybe rake task?

@jacobvosmaer btw your concept with pre-receive hooks is awesome. I am sure I can make it work

@dzaporozhets about pre-receive hooks: yay :)

About rake tasks: I think what @bbodenmiller is saying is that it is nice if we can avoid introducing a manual 'clean up satellites' step.

I am not sure though if creating a sidekiq job from a DB migration is smart, it needs Redis access.

@jacobvosmaer I dont think creating separate sidekiq worker for one time action makes sense. Also people might not expect hdd load in hour after migraiton. I rather say users explicitly - please remove this directory when you feel comfortable with it. Its not necessary anymore

Maybe we can include a rake task to clean then in 8.0. Then in 8.1 we have a migration call that rake task. If they are removed already it will be fast.

@dzaporozhets about the pre-receive hook: forget what I wrote about writing to files in refs/heads/xxx, it is probably better to use git update-ref. They even say it in the man page:

In general, using

git update-ref HEAD "$head"

should be a lot safer than doing

echo "$head" > "$GIT_DIR/HEAD"

In our case it would be git update-ref $branch $newrev $oldrev

Added 12 commits:

• 34690142 - Implement commit transaction with pre-receive and post-receive hooks for web editor
• c8614cbb - Capture pre-receive exception
• 9011a322 - Disable pre-receive check in test env
• 9649f938 - Fix tests for web editor
• bacad39e - Make sure target has not changed during pre-receive hook
• 4e4866f2 - Refactor pre/post receive commit services into one class
• f4149bcd - Refactor how repository makes commit with pre/post receive hooks
• 9ea37cda - merge with support of pre-receive and post-receive hooks
• a608e45e - Remove unnecessary file
• 3b93856e - save repo empty state into variable for proper result
• e02cff19 - Fix typo in text when raise pre-receive exception
• bbe0e58a - Merge branch 'pre-receive-wo-satellites' into 'remove-satellites'

Added 53 commits:

• bbe0e58a...9f7c7c85 - 52 commits from branch master
• 4a0fa270 - Merge branch 'master' into remove-satellites

Status changed to merged

mentioned in issue #2147 (closed)

We forgot to make the migration :(

@rspeicher will make it in a patch release for 8.1

And report back in https://about.gitlab.com/2015/10/22/gitlab-8-1-released/#comment-2321843410

@dzaporozhets @jacobvosmaer Did we settle on the best method for doing this? I'm leaning towards manual rake task, personally.

@rspeicher I think rake task is most reasonable way to do with it.

People will forget to run the rake task.

The rake task cannot stay in the GitLab source forever, unless we keep the satellite config option around too. Otherwise we do now know what directory to delete.

Per @JobV : Rake task in 8.1 patch, migration in 8.2.

Milestone changed to 8.1

And comments, both in gitlab and in omnibus, with warnings not to remove the code that manages the gitlab-satellites directory location. Because the migration that deletes the satellite may run from gitlab 8.35 in the future.

@rspeicher We promised this for 8.1 and will ship it for 8.1. Please create a migration and ship it in a patch release. In the blog post announcing the patch release include code to manually download the directories in case you want to do a quicker deploy: rm -rf /var/opt/gitlab/git-data/gitlab-satellites.

Also it requires mention in 8.1 release post too. People might not reading patch release blog post because its patch release and they expect smooth update

Agree with mentioning it in 8.1 release post too (with a link to the patch release).

mentioned in merge request !7237 (merged)