Prune outdated refs/tmp references

From gitlab-com/infrastructure#2904, we appear to have over 8,000 temporary references in the GitLab CE repository that probably don't need to be there anymore. This seems to be slowing pushes by 20+ seconds.

As part of our garbage collection process, we should probably remove these if they are older than some time (e.g. a week) because they can significantly affect performance. These refs aren't folded into the packed-refs file, so each of them will cause an extra stat and open system call unnecessarily.

Note that these refs aren't even valid refs; they are empty directories, so there's even more reason to delete them.

/cc: @mydigitalself, @DouweM

@godfat and @rymai saw this elsewhere (on a different repo)!

https://gitlab.slack.com/archives/C4N66DV1S/p1506582362000077

@stanhu do you mean they slow ANY push to any repo down by 20 seconds, or just CE?

Nevermind, didn't read the issue properly.

Is this a dupe of https://gitlab.com/gitlab-org/gitlab-ce/issues/30639 ?

@mydigitalself No, this is not related to LFS.

@godfat I know that you looked into pruning refs before; how easy would it be to do this?

To be clear, these refs don't exist - at least in the cases I've seen - but their directories still exist. My guess is because of the directory structure we use: I'm not sure if git will delete refs/foo when deleting refs/foo/bar, even if refs/foo is now empty.

Maybe we should switch to using refs/tmp/$sha, and leave off the trailing /head?

mentioned in issue #38332 (moved)

@smcgivern Right, good point, they are empty directories that don't get packed into the packed-refs file because they're not valid git refs.

Well, there are a couple of things:

1. They don't get packed because we actually remove them immediately: https://gitlab.com/gitlab-org/gitlab-ce/issues/38332#note_42243357
2. We're then left with these empty directories that still get scanned.

In an ideal world, I think refs/tmp would be empty almost all of the time.

@stanhu how many temp refs do we currently have in this repo? How many are empty?

@stanhu FYI we discovered the exact same thing that last Friday, and I think housekeeping should indeed take care of that.

I think https://gitlab.com/gitlab-org/gitlab-ce/issues/38498 is more generic since it's about removing empty directories under refs/ when housekeeping? We don't even need to check their mtime or filter only refs/tmp / refs/merge-requests, we can just delete all empty directories in refs/**/*.

@DouweM I think there are probably 3 related topic here:

0. Empty directories in general as described in #38498 (moved)
1. Empty directories in refs/tmp
2. Unused refs in refs/tmp (Do we have this issue?)

And:

0. For empty directories in general, we need to consider about race conditions so it could be harder to do it right.
1. For empty directories in refs/tmp I think we could safely just remove them, as they shouldn't be used at the same time, and we should do what @smcgivern suggested in https://gitlab.com/gitlab-org/gitlab-ce/issues/38689#note_42242988 so that we just use refs/tmp/HEX and don't create potential empty directories. As for when we should do this cleanup, maybe also in housekeeping.
2. For unused refs in refs/tmp, we should probably clean them up as @stanhu suggested that we remove refs which are old enough, assuming that they're just left as garbage. We could also do this when housekeeping, assuming that people are NOT using refs/tmp in their workflow.

@godfat Thanks for breaking it down! So:

1. is being taken care of in https://gitlab.com/gitlab-org/gitlab-ce/issues/38498
2. we can fix by using refs/tmp/HEX instead of refs/tmp/HEX/head, and the existing empty dirs will be cleaned up as part of 1.
3. we don't know is actually a problem. We can run git for-each-ref 'refs/tmp' to see how many tmp refs we currently have alive. I'd expect a handful for currently running background tasks like merges, but not too many.

@godfat I think there won't be any unused refs in there, because of the ensure that deletes them (that we're discussing in the other issue!).

@jramsay one for you to consider for future planning.

mentioned in commit e25abf5b

mentioned in merge request !14698 (merged)

I made the MR to rename the path: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14698

@smcgivern Yeah, I was just thinking that there might have some errors causing it left some garbage. Also, IIRC, we didn't try to delete them before.

Thanks @godfat! I commented on the MR, but maybe it's better here:

How do we clean up the empty directories for people? (Or for repos in general.)

Could we add something to housekeeping?

marked this issue as related to #38498 (moved)

@smcgivern As summarised at https://gitlab.com/gitlab-org/gitlab-ce/issues/38689#note_42346248

we can fix by using refs/tmp/HEX instead of refs/tmp/HEX/head, and the existing empty dirs will be cleaned up as part of 1.

So yes, I think we basically try to clean them up at housekeeping https://gitlab.com/gitlab-org/gitlab-ce/issues/38498

@godfat right, I'd already looked at that issue, too

changed milestone to %10.3

FYI, I think this became a problem on the www-gitlab-com repository. We had 5971 refs/tmp there as well.

changed the description

For reference, @stanhu's comments came from a discussion about slowness on git pushes to www-gitlab-com, here: https://gitlab.slack.com/archives/C1GDKLKEV/p1517331238000335

@jramsay, do you think it would be worth scheduling this fix for an upcoming release?

mentioned in issue #33526 (closed)

Thanks @andrewn @stanhu - yes, I'd really like to schedule this soon!

mentioned in issue #39248 (closed)

mentioned in merge request gitaly!587 (merged)

I'm investigating things we can do to improve Geo replication, and removing this could help us a lot:

grep -c refs/tmp packed-refs
5860

we are now close to 6K

added [deprecated] Accepting merge requests label

added Create [DEPRECATED] label

@stanhu @DouweM et al

We ran into a serious performance issue with a customer. Using strace we found that loading /dashboard/projects the first time was calling stat on 18,000 files, most of which were in refs/tmp/. Cleaning up those files mostly resolved the performance issue.

• Is it safe to clean up these temp refs? I guess the only concern would be any in-flight merges or rebases may fail?

Customer @dblessing is referring to: https://gitlab.my.salesforce.com/0016100001Niizb - cc @jramsay

added customer+ label

Is it safe to clean up these temp refs? I guess the only concern would be any in-flight merges or rebases may fail?

Are the sizes of these refs empty? You can delete them if they are. You can probably delete them too if they are older than an hour.

@DouweM It looks like we create a temp ref and delete it every time we do a branch comparison. I can see needing to do that for forks, but is it really necessary when the comparison happens within the same project?

@jacobvosmaer-gitlab given the conversation the other day about safe GC and pruning, do you have any thoughts?

removed 1 deleted label

is it really necessary when the comparison happens within the same project?

@stanhu It definitely isn't. This looks like a performance regression introduced by https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19700. Before, compare_source_branch used with_repo_branch_commit which would only create a temp ref when the repositories are different and the commit is not already in the current repository, but the new implementation lacks that logic and will always create a temp ref.

added backend label

I'm not sure what problem we're talking about now because this issue is a year old and I see multiple problems coming by.

Empty files under refs/ are by definition broken and should be deleted. We recently added cleanup code for such empty files (gitaly!992 (merged)) but I'm not sure if that covers refs/tmp. cc @reprazent

---

Edit: gitaly!992 (merged) only covers refs/keeparound

mentioned in commit dee9915a

mentioned in commit 7579cf22

mentioned in merge request !24038 (merged)

mentioned in commit 533e5e54

mentioned in commit b60b8f34

mentioned in commit 2fb4876c

mentioned in commit 744bb7a9

mentioned in commit 26653eb0

mentioned in commit 9162f4fc

mentioned in commit 401f494b

mentioned in commit ad682663

added devopscreate groupsource code labels

added groupgitaly label and removed groupsource code + 1 deleted label

This appears to still be an issue. For the CE repository, we have 6,523 tmp/refs all in packed-refs now too:

# cat packed-refs | grep "refs/tmp" | wc -l
6523

mentioned in issue #66545 (closed)

Hey!

GitLab is moving all development for both GitLab Community Edition and Enterprise Edition into a single codebase. The current gitlab-ce repository will become a read-only mirror, without any proprietary code. All development is moved to the current gitlab-ee repository, which we will rename to just gitlab in the coming weeks. As part of this migration, issues will be moved to the current gitlab-ee project.

If you have any questions about all of this, please ask them in our dedicated FAQ issue.

https://gitlab.com/gitlab-org/gitlab-ee/issues/13855

You can also find more information here:

• https://gitlab.com/gitlab-org/gitlab-ee/issues/13304
• https://about.gitlab.com/2019/08/23/a-single-codebase-for-gitlab-community-and-enterprise-edition/

Frequently Asked Questions

For an up to date list of questions and answers, please take a look at https://gitlab.com/gitlab-org/gitlab-ee/issues/13855

What will we do with the repository names?

https://gitlab.com/gitlab-org/gitlab-ee/ will become https://gitlab.com/gitlab-org/gitlab, and https://gitlab.com/gitlab-org/gitlab-ce will become https://gitlab.com/gitlab-org/gitlab-foss.

Why rename gitlab-ce to gitlab-foss?

Using "gitlab" and "gitlab-ce" would be confusing, so we decided to rename gitlab-ce to gitlab-foss to make the purpose of this FOSS repository more clear

I created a merge requests for CE, and this got closed. What do I need to do?

You will need to create a merge request at https://gitlab.com/gitlab-org/gitlab-ee/. This link will eventually redirect to https://gitlab.com/gitlab-org/gitlab.

How does the licensing work in this new setup?

Everything in the ee/ directory is proprietary. Everything else is free and open source software. If your merge request does not change anything in the ee/ directory, the process of contributing changes is the same as when using the gitlab-ce repository.

Will you accept merge requests on the gitlab-ce/gitlab-foss project after it has been renamed?

No. Merge requests submitted to this project will be closed automatically.

Will I still be able to view old issues and merge requests in gitlab-ce/gitlab-foss?

Yes.

How will this affect users of GitLab CE using Omnibus?

No changes will be necessary, as the packages built remain the same.

How will this affect users of GitLab CE that build from source?

Once the project has been renamed, you will need to change your Git remotes to use this new URL. GitLab will take care of redirecting Git operations so there is no hard deadline, but we recommend doing this as soon as the projects have been renamed.

Where can I see a timeline of the remaining steps?

https://gitlab.com/gitlab-org/gitlab-ee/issues/13304

Will community contributions submitted to the new "gitlab" repository be available in the new gitlab-foss repository?

Yes, these changes will be synced to gitlab-foss automatically, several times per day.

locked this issue