Invalid state of snippets after Project import
I've been checking production lately and, after running the snippet background migration (#208904 (closed)), and I've noticed that every week we have new snippets without repositories. Some times I've run manually the migration and again, new snippets without repositories appear.
The thing is that looking at Kibana, we don't see the same patterns that we might have when the user creates snippets through the snippet create/update services.
In fact, an interesting thing is that those non-migrated snippets ids are quite high meaning that are new snippets. So how are those snippets created and why the repository is not created?
Today I discovered that all non-migrated snippets are project ones. Besides, I've seen that some of them have correlative ids, meaning that they were created one after another. Another interesting fact is that those non-migrated snippets had old
created_at dates not up to dates ones like they should.
All these facts led me to think that this problem was related to project imports. Once I checked some of these snippets' projects, I could see that these were imported projects and they all had import errors.
Therefore, this is the main problem. When we import snippets, we import the db records first and after we call the
SnippetRepoLoader to restore the snippet contents. The problem is that if there is an error in any of the other
loaders, snippets won't have a repository and this is a state we don't want and shouldn't be present. It's better to delete the snippet rather than having it in the database without a repository.
Now, it sounds hard because we have all the information we need to create the snippet repository. Nevertheless, one we add the ability to add multiple files per snippets, that info won't be stored in the database, so we don't need to create even the record in the database and just fail the process.
I think we have to act in two places:
SnippetsRepoRestorer is called but one of the snippet repositories couldn't be created.
In https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/import_export/snippets_repo_restorer.rb#L15 we call the
SnippetRepoRestorer per each snippet. Whenever one of those calls returns
false, the process stops and we won't try to backfill the rest of snippets. This means that we will end up with non migrated snippets.
We can tweak this a little bit and call the
SnippetRepoRestorer service on every snippet, no matter the result of previous snippets. Then, if any of the snippets failed to create the repository we will just fail the operation and delete those snippets that couldn't be migrated. This will prevent us from having non-migrated snippets in the database.
SnippetsRepoRestorer is not even called
In https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/import_export/importer.rb#L21, we call all the object restorers one by one but, what happens if any of them raises an unexpected exception. Well, if the restorer is before in line that the
SnippetsRepoRestorer that means that no snippet will be migrated and we will have non migrated snippet in the database. My proposal here is to call the
SnippetRepoRestorer when the exception is raised. It will take care of removing those snippet that couldn't be migrated.