Renaming project files causes both the new and old path to display in search results

Summary

With Elasticsearch enabled for Advanced Global Search, renaming wikis and other code files results in both the old path and the new path shown in the search results. This is confusing as only the new path is in the current wiki or repository, but the search results display as if the old path is current.

Customer ticket reporting this issue: https://gitlab.zendesk.com/agent/tickets/133979 (internal use only)

Steps to reproduce

You have Elasticsearch enabled for your GitLab instance.

1. Create a wiki in a project. Title it Wiki and add some content, i.e. This is a wiki
2. Search globally for the term wiki, you should see one search result for the wiki you just created.
3. Change the title of the wiki to Blog, keep the content the same and save.
4. Search globally for the term wiki, see two search results for the wiki you have created, one with the new path to Blog and one with the old path to Wiki

If I query the rails console (sudo gitlab-rails console) to see what Elasticsearch is returning, I get:

user = User.find_by_username('root')
s = SearchService.new(user, {:search => 'wiki', :scope => 'wiki_blobs'})
pp s.search_objects.to_a

# Result
[#<Elasticsearch::Model::Response::Result:0x00007f588b661760
  @result=
   {"_index"=>"gitlab-production",
    "_type"=>"doc",
    "_id"=>"188_Wiki.md",
    "_score"=>10.960577,
    "_routing"=>"project_188",
    "_source"=>
     {"blob"=>
       {"type"=>"wiki_blob",
        "oid"=>"13db843fd38b5bd253da3c00901a97d43bfae1fe",
        "rid"=>"wiki_188",
        "commit_sha"=>"f20b600736d1b22f56298f47d09ff6eaa04f77aa",
        "content"=>"This is a wiki",
        "path"=>"Wiki.md",
        "file_name"=>"Wiki.md",
        "language"=>"Markdown"},
      "join_field"=>{"name"=>"wiki_blob", "parent"=>"project_188"},
      "project_id"=>188,
      "type"=>"wiki_blob"},
    "highlight"=>
     {"blob.file_name"=>["gitlabelasticsearch→Wiki.md←gitlabelasticsearch"],
      "blob.content"=>
       ["This is a gitlabelasticsearch→wiki←gitlabelasticsearch"]}}>,
 #<Elasticsearch::Model::Response::Result:0x00007f588b669f00
  @result=
   {"_index"=>"gitlab-production",
    "_type"=>"doc",
    "_id"=>"188_Blog.md",
    "_score"=>4.559907,
    "_routing"=>"project_188",
    "_source"=>
     {"blob"=>
       {"type"=>"wiki_blob",
        "oid"=>"13db843fd38b5bd253da3c00901a97d43bfae1fe",
        "rid"=>"wiki_188",
        "commit_sha"=>"3602bccae92f57648d555886413ca87decb40aed",
        "content"=>"This is a wiki",
        "path"=>"Blog.md",
        "file_name"=>"Blog.md",
        "language"=>"Markdown"},
      "join_field"=>{"name"=>"wiki_blob", "parent"=>"project_188"},
      "project_id"=>188,
      "type"=>"wiki_blob"},
    "highlight"=>
     {"blob.content"=>
       ["This is a gitlabelasticsearch→wiki←gitlabelasticsearch"]}}>]

You can also do these steps for a code file in the project repository in addition to the wiki.

1. Create a file called gitlab.rb in your project and add some content, i.e. external_url: example.com.
2. Search globally for the term example.com, you should see one search result for the file you just created.
3. Change the title of the file to gitlap.rb, keep the content the same and save.
4. Search globally for the term example.com, see two search results for the file you have created, one with the new path to gitlap.rb and one with the old path to gitlab.rb

Example Project

I was not able to re-create this issue on GitLab.com

What is the current bug behavior?

When you rename a wiki or code file, the search results will display two results when searching for that file, one with the old path and one with the new path.

What is the expected correct behavior?

When you rename a wiki or code file, the search results should display one result when searching for that file, the result with the new path.

Output of checks

Results of GitLab environment info

Expand for output related to GitLab environment info

System information
System:		Ubuntu 16.04
Proxy:		no
Current User:	git
Using RVM:	no
Ruby Version:	2.6.3p62
Gem Version:	2.7.9
Bundler Version:1.17.3
Rake Version:	12.3.2
Redis Version:	3.2.12
Git Version:	2.22.0
Sidekiq Version:5.2.7
Go Version:	go1.6.2 linux/amd64
GitLab information
Version:	12.3.3-ee
Revision:	47e969b011f
Directory:	/opt/gitlab/embedded/service/gitlab-rails
DB Adapter:	PostgreSQL
DB Version:	10.7
URL:		http://198.199.92.126.xip.io
HTTP Clone URL:	http://198.199.92.126.xip.io/some-group/some-project.git
SSH Clone URL:	git@198.199.92.126.xip.io:some-group/some-project.git
Elasticsearch:	yes
Geo:		no
Using LDAP:	yes
Using Omniauth:	yes
Omniauth Providers: saml, gitlab, google_oauth2
GitLab Shell
Version:	10.0.0
Repository storage paths:

• default: /var/opt/gitlab/git-data/repositories GitLab Shell path: /opt/gitlab/embedded/service/gitlab-shell Git: /opt/gitlab/embedded/bin/git

Results of GitLab application Check

Expand for output related to the GitLab application check

Checking GitLab subtasks ...
Checking GitLab Shell ...
GitLab Shell: ... GitLab Shell version >= 10.0.0 ? ... OK (10.0.0)
Running /opt/gitlab/embedded/service/gitlab-shell/bin/check
Check GitLab API access: OK
Redis available via internal API: OK
gitlab-shell self-check successful
Checking GitLab Shell ... Finished
Checking Gitaly ...
Gitaly: ... default ... OK
Checking Gitaly ... Finished
Checking Sidekiq ...
Sidekiq: ... Running? ... yes
Number of Sidekiq processes ... 1
Checking Sidekiq ... Finished
Checking Incoming Email ...
Incoming Email: ... Reply by email is disabled in config/gitlab.yml
Checking Incoming Email ... Finished
Checking LDAP ...
LDAP: ... Server: ldapmain
Exception: Connection timed out - user specified timeout
Checking LDAP ... Finished
Checking GitLab App ...
Git configured correctly? ... yes
Database config exists? ... yes
All migrations up? ... yes
Database contains orphaned GroupMembers? ... no
GitLab config exists? ... yes
GitLab config up to date? ... yes
Log directory writable? ... yes
Tmp directory writable? ... yes
Uploads directory exists? ... yes
Uploads directory has correct permissions? ... yes
Uploads directory tmp has correct permissions? ... yes
Init script exists? ... skipped (omnibus-gitlab has no init script)
Init script up-to-date? ... skipped (omnibus-gitlab has no init script)
Projects have namespace: ...
46/1 ... yes
46/2 ... yes
46/3 ... yes
46/4 ... yes
47/5 ... yes
47/6 ... yes
47/8 ... yes
47/9 ... yes
47/10 ... yes
47/11 ... yes
48/12 ... yes
48/13 ... yes
48/14 ... yes
48/15 ... yes
48/16 ... yes
48/17 ... yes
49/18 ... yes
49/19 ... yes
49/20 ... yes
49/21 ... yes
49/22 ... yes
49/23 ... yes
49/24 ... yes
49/25 ... yes
49/26 ... yes
49/27 ... yes
50/28 ... yes
50/29 ... yes
50/30 ... yes
50/31 ... yes
50/32 ... yes
50/33 ... yes
50/34 ... yes
51/35 ... yes
51/36 ... yes
51/37 ... yes
51/38 ... yes
51/39 ... yes
51/40 ... yes
51/41 ... yes
51/42 ... yes
51/43 ... yes
52/44 ... yes
52/45 ... yes
52/46 ... yes
52/47 ... yes
52/48 ... yes
52/49 ... yes
52/50 ... yes
52/51 ... yes
53/52 ... yes
53/53 ... yes
53/54 ... yes
53/55 ... yes
53/56 ... yes
53/57 ... yes
53/58 ... yes
53/59 ... yes
54/60 ... yes
54/61 ... yes
54/62 ... yes
55/63 ... yes
55/64 ... yes
56/65 ... yes
56/66 ... yes
56/67 ... yes
57/68 ... yes
57/69 ... yes
57/70 ... yes
57/71 ... yes
57/72 ... yes
57/73 ... yes
58/74 ... yes
58/75 ... yes
58/76 ... yes
58/77 ... yes
58/78 ... yes
59/79 ... yes
59/80 ... yes
59/81 ... yes
59/82 ... yes
59/83 ... yes
59/84 ... yes
59/85 ... yes
60/86 ... yes
60/87 ... yes
60/88 ... yes
60/89 ... yes
60/90 ... yes
1/91 ... yes
60/92 ... yes
1/93 ... yes
83/94 ... yes
83/95 ... yes
83/96 ... yes
84/97 ... yes
84/98 ... yes
84/99 ... yes
84/100 ... yes
85/101 ... yes
85/102 ... yes
85/103 ... yes
86/104 ... yes
86/105 ... yes
86/106 ... yes
86/107 ... yes
86/108 ... yes
87/109 ... yes
87/110 ... yes
88/111 ... yes
88/112 ... yes
88/113 ... yes
89/114 ... yes
89/115 ... yes
90/116 ... yes
90/117 ... yes
90/118 ... yes
90/119 ... yes
91/120 ... yes
91/121 ... yes
91/122 ... yes
91/123 ... yes
91/124 ... yes
91/126 ... yes
110/127 ... yes
60/128 ... yes
60/129 ... yes
60/130 ... yes
1/131 ... yes
1/132 ... yes
1/133 ... yes
1/134 ... yes
1/135 ... yes
1/136 ... yes
1/137 ... yes
60/138 ... yes
111/139 ... yes
1/140 ... yes
1/141 ... yes
1/142 ... yes
104/143 ... yes
1/144 ... yes
1/145 ... yes
1/146 ... yes
1/147 ... yes
114/148 ... yes
1/149 ... yes
1/150 ... yes
117/151 ... yes
118/152 ... yes
119/153 ... yes
120/154 ... yes
60/155 ... yes
1/156 ... yes
1/157 ... yes
60/158 ... yes
121/159 ... yes
91/160 ... yes
1/161 ... yes
128/162 ... yes
1/163 ... yes
1/164 ... yes
121/165 ... yes
115/166 ... yes
115/167 ... yes
131/168 ... yes
1/169 ... yes
133/170 ... yes
1/171 ... yes
7/172 ... yes
70/173 ... yes
129/174 ... yes
135/175 ... yes
70/176 ... yes
157/177 ... yes
157/178 ... yes
120/179 ... yes
159/180 ... yes
1/181 ... yes
1/182 ... yes
176/187 ... yes
159/188 ... yes
179/189 ... yes
Redis version >= 2.8.0? ... yes
Ruby version >= 2.5.3 ? ... yes (2.6.3)
Git version >= 2.22.0 ? ... yes (2.22.0)
Git user has default SSH configuration? ... yes
Active users: ... 81
Is authorized keys file accessible? ... yes
Elasticsearch version 5.6 - 6.x? ... yes (6.6.1)
Checking GitLab App ... Finished
Checking GitLab subtasks ... Finished

added advanced search customer groupglobal search typebug labels

@lulalala @toupeira thoughts on this one?

/cc @dsatcher

@phikai I can reproduce this on a local instance, tentatively assigning ~P3 ~S3. It looks like only blobs are affected, I assume for DB records we use their ID to uniquely identify them.

Not sure where this needs to be fixed though, probably in the Go indexer? @nick.thomas might be able to point us in the right direction.

The Go indexer should generate a diff that tells it to remove the old elasticsearch document for the blob, and add the new elasticsearch document. It does this based on a pair of SHAs provided to it by gitlab in the environment. We run the command here: https://gitlab.com/gitlab-org/gitlab/blob/master/ee/lib/gitlab/elastic/indexer.rb

It would be worth seeing if you can replicate this for repository blobs. The code for the two is very similar.

Perhaps we generate the elasticsearch document ID, or overall remove command, incorrectly when the type is wiki_blob?

This may be a regression - we only recently moved wiki indexing from the Ruby to Go implementation.

We can see in https://gitlab.com/gitlab-org/gitlab-elasticsearch-indexer/blob/master/indexer/indexer.go#L87 we use the same method (removeBlob) for both wiki and repo blobs. That would be the first thing I'd dig into!

It would be worth seeing if you can replicate this for repository blobs. The code for the two is very similar.

@nick.thomas I did reproduce it in a normal repository, sorry for not mentioning that. With wikis it's the same behaviour.

Looking at Repository.EachFileChange, it seems the problem is that we're only deleting blobs (by calling del / i.removeBlob) when a file is deleted, but not when renamed (where we just call mod / i.submitRepoBlob).

Could we fix this by always deleting blobs for modified files first before indexing the new content, i.e. calling mod() then del()?

@toupeira so this affects searches for the normal git repository in addition to searches in the wiki? That sounds a bit more than an ~S3 if so ^^.

If mod is always del(); add(), then we could just remove it entirely.

@toupeira so this affects searches for the normal git repository in addition to searches in the wiki? That sounds a bit more than an ~S3 if so ^^.

Yes, just confirmed it again. It was also mentioned in the original report:

renaming wikis and other code files

At least the search results won't 404 because they're linked to the commit where the search term last appeared, which is different to wiki pages which are always linked to master.

Not sure if this warrants ~S2? The workaround seems to be reindexing everything, which is definitely not always acceptable

If mod is always del(); add(), then we could just remove it entirely.

Ok yeah I think what I meant was doing del() with the old path and add() with the new path. But this sounds like the diffing you mentioned in the Go indexer, so maybe some form of this is already supposed to happen?

Setting devopsenablement ~"Category:Search" based on ~"group::search".

added devopssystems label

added priority3 severity3 labels

mentioned in merge request !18051 (merged)

mentioned in merge request gitlab-elasticsearch-indexer!41 (merged)

assigned to @DylanGriffith

mentioned in issue #12844

changed milestone to %12.4

mentioned in commit 15c3155b

mentioned in commit 81e0f218

closed via merge request !18558 (merged)

mentioned in commit d51133ad

mentioned in commit Dalahro/gitlab@4d1a3d31

added devopsdata stores label and removed devopssystems label