fix: delete stale chunks after incremental runs
What does this MR do and why?
After an incremental run, the index could keep content that no longer exists, or miss content that does. Files that became binary or empty kept their chunks. Type changes, such as a symlink that became a file, were ignored. Large deletes and batches of large files failed on Elasticsearch and OpenSearch query limits. This MR removes the stale chunks, indexes type changes, and keeps each delete query within the limits.
Related issues
Closes #205 (closed).
Related to gitlab!258433 (merged). After that MR, a force push indexes the diff from the last indexed commit. Before, a full re-index removed these stale chunks as a side effect.
Testing
- Unit tests for each fix, and live tests for the query limits. Each live test fails with the code from
mainand passes with this branch. make testpasses on Elasticsearch 9.2.1 and OpenSearch 2.10.- Checked in GDK with Elasticsearch and PostgreSQL (
pgvector), in semantic code search and in advanced search. See the agent context for the results.
Performance analysis
An incremental batch with binary or empty files sends one more delete request. Initial indexing and forced re-indexing send no extra requests. Large deletes and orphan cleanup are split into several smaller requests.
- This merge request does not introduce a performance regression. If a regression is expected, explain why.
Agent context - long-form analysis, file-by-file walkthroughs, profiler output, and alternatives considered
Stale chunks of files with no content. BulkIndex skips binary files, and the chunker returns no chunks for an empty file. Orphan cleanup only covers paths that produced chunks, so these paths kept their old chunks. BulkIndex now deletes the chunks of the paths in the batch that produced no chunks. It skips this when from_sha is blank: initial indexing has no old chunks, and a forced re-index removes them in ResolveReindexing. The change is in chunk.go, so it applies to all adapters, including PostgreSQL.
Type changes, also for advanced search. getChangedPaths handled only ADDED, MODIFIED and DELETED, and skipped any change with a submodule on either side. Gitaly reports TYPE_CHANGE for file to symlink, symlink to file, and file to or from submodule, so these changes were only logged. It now decides by content: if the new side is a file or symlink, index the path; else if the old side was, delete it; else skip. Submodule content is still never indexed. getChangedPaths is shared with advanced search, which gets the same fix. Normal changes behave as before.
Query limits. Elasticsearch and OpenSearch reject a terms query with more than 65,536 values (index.max_terms_count). Elasticsearch 9.4 and later also reject an ids query with more than 10,000 values (index.max_result_window); 9.2 and 9.3 accept it.
DeletePathssent all deleted paths in one query. It now sends batches of 1,000 paths.- Orphan cleanup sent one query with all paths and chunk IDs of a batch. The path list had one entry per chunk, so a batch of large files passed the
termslimit. It now groups the chunk IDs by path and sends batches of at most 1,000 paths and 10,000 IDs. - PostgreSQL uses
ANYand has no such limit.
Known limit: one file with more than 10,000 chunks still fails orphan cleanup on Elasticsearch 9.4 and later. With the 1 MiB file limit, this needs a chunk size below about 100 bytes. If gitlab#631147 decides to index whole files, this needs to be handled.
QA in GDK. Each case compares the indexer from main (OLD) with this branch (NEW), with an initial run and then an incremental run.
| Case | OLD | NEW |
|---|---|---|
| File became binary, file became empty (Elasticsearch, PostgreSQL) | old chunks stay | no chunks left |
| File to submodule (semantic and advanced search) | old content stays | removed |
| File to symlink | old content stays | indexed as the symlink |
| Symlink to file | new file not indexed | indexed |
| Submodule to file | new file not indexed | indexed |
| 70,000 paths that share one blob | run fails with Error 400 |
run succeeds |