Index group milestones in Advanced Search and keep them in sync
What does this MR do and why?
This MR indexes group milestones in Elasticsearch and keeps their documents in sync with their group.
- The group milestone document adds the group's
namespace_id(the group id),traversal_ids,visibility_level, andarchived(build_group_datainee/lib/elastic/latest/milestone_instance_proxy.rb).namespace_idis already mapped as an integer in the main index, so no mapping migration is needed. It's there so deletes can target a group's milestones. - Group milestone documents are routed by
n_<root_ancestor_id>(es_parent), usinggroup.traversal_ids.firstto avoid aroot_ancestorquery per group. Project milestones keepproject_<id>routing. - Group milestone documents have no
join_field. Master stopped writing it in 4f7ac7db, and this MR is rebased on that. Routing still comes fromes_parent. Removingjoin_fieldfrom the mapping is tracked in #630682. There's noSCHEMA_VERSIONbump: project milestone documents are unchanged, and group milestone documents are new, so no migration needs to find old versions. Milestone.preload_for_indexingpreloads the group's namespace settings, so readingarchiveddoesn't cost a query per milestone. Thedoes not have N+1 queries for group milestonesspec inee/spec/services/elastic/process_bookkeeping_service_spec.rbcovers this: without thegroup:preload it goes from 5 to 11 queries.Milestone#use_elasticsearch?(EE) now returnsproject.use_elasticsearch?for project milestones andgroup.use_elasticsearch?for group milestones. It used to fall back toproject&.use_elasticsearch?, which isnilfor group milestones, so they were never indexed. From this MR on, new and updated group milestones are indexed through the normal callbacks.- Sync:
- Delete:
Search::ElasticGroupAssociationDeletionWorkernow also runs adelete_by_queryon the main index for group milestone documents, routed byn_<ancestor_id>and filtered ontype: milestone,namespace_idin the group ids, and atraversal_idsprefix<ancestor_id>-. The prefix matters because routing only picks the shard: without it, a delete after a transfer to a new root could also remove documents already reindexed under the new root, if both routings land on the same shard. The same gap in the work item delete is fixed separately in !257721 (merged). The worker already runs on group delete (milestones.group_idisON DELETE CASCADE, so rows disappear without callbacks), on transfer to a new root (with descendants), and on removal from limited indexing. - Visibility change and newly-enabled indexing:
Elastic::ProcessBookkeepingService.maintain_indexed_namespace_associations!now includesmilestonesby default for group namespaces, likeepics. This makes the existing callers reindex group milestones: the visibility change callback inElastic::MaintainElasticsearchOnGroupUpdate,ElasticNamespaceIndexerWorkerwhen a namespace is added to limited indexing, andElastic::NamespaceUpdateWorkeron parent change. - Transfer:
EE::Groups::TransferService#process_group_associationsreindexes the group milestones of the transferred group and its descendants when the group uses Elasticsearch, so they pick up the newtraversal_idsand routing. Wikis, epics, and milestones now share a singlegroup.use_elasticsearch?check there.process_wikismoved inside it frompost_update_hooksand lost its own check. This doesn't change behaviour, becauseGroup#use_elasticsearch?is already false when indexing is off. Epics and milestones are tracked in one pass over the group's descendants. - Archive and unarchive:
Search::Elastic::GroupArchivedEventWorker(handlesNamespaces::Groups::GroupArchivedEvent, published by both services) also tracks the group milestones of the group and its descendants that use Elasticsearch. - Rake tasks:
gitlab:elastic:indexalready covers group milestones throughElasticNamespaceIndexerWorker, which callsmaintain_indexed_namespace_associations!.gitlab:elastic:index_group_entitiesnow also indexes group milestones, walking groups in batches and respecting limited indexing (including subgroups). The rake task docs are updated. - Group import: direct transfer and file-based group import save milestones with
save!, so the create callback indexes them when the destination group uses Elasticsearch. No change was needed there. - New scope
Milestone.of_groups(ids), alongsideof_projects.
- Delete:
- Search results don't change yet: the milestone search filter requires issue and merge request access level fields, which group milestone documents don't have. #630618 adds those. A spec covers that a new group milestone is indexed but not returned for a member or an admin.
Backfilling existing group milestones is a separate issue, #630857, that ships after this MR is deployed.
References
- Closes #630654 (closed)
- Related to #428589
- Implementation plan: #428589 (comment 3076846649)
- Backfill follow-up: #630857
- Query builder follow-up: #630618
- Work item delete fix: !257721 (merged)
- Related to !125137 (merged) (prior art:
n_<root_ancestor_id>routing for wikis)
Screenshots or screen recordings
No UI change. This MR is backend-only.
How to set up and validate locally
- Enable Advanced Search indexing in GDK.
- Create a group milestone and check its document:
_routingisn_<root group id>, and it hasnamespace_id,traversal_ids,visibility_level, andarchived. - Archive the group and check
archivedbecomestrueon the document after indexing. Change the group's visibility and checkvisibility_levelupdates. - Transfer a subgroup that has a milestone to another root and check that exactly one document remains, routed by the new root.
- Delete the group and check the document is gone.
- Delete the milestone documents from the index, run
bundle exec rake gitlab:elastic:index_group_entities, and check that the group milestones are indexed again once the bookkeeping queue is processed. - Search at the group level and check the milestone isn't returned yet. This is expected until #630618 lands.
Database
This MR adds no schema change and no database migration. It only adds read queries on milestones (about 2.8M rows on GitLab.com). The ReindexMilestones advanced search migration was removed. Backfilling is tracked in #630857.
The database reviewer bot reviewed this MR at 08b242fa (note) and found nothing blocking. Its one improvement, that the transfer service should pluck ids, is applied in ae3ca02c.
Relevant indexes: index_milestones_on_group_id (btree on group_id) and milestones_pkey. There is no (group_id, id) index.
New scopes on Milestone: of_groups(ids) is where(group_id: ids). with_group is preload(:group). order_by_group_and_id is order(:group_id, :id).
1. Group transfer and gitlab:elastic:index_group_entities
Both callers read milestones with Gitlab::Pagination::Keyset::Iterator over Milestone.of_groups(...).with_group.order_by_group_and_id, which replaced each_batch(order_hint: :group_id). EE::Groups::TransferService#process_epics_and_milestones and Search::RakeTaskExecutorService#index_group_milestones both pluck each batch's ids first with pluck_primary_key, because a subquery is repeated in each of the four UNION ALL branches on every keyset page. The pluck alone was not enough for the rake task. Under limited indexing, the batch came from elasticsearch_enabled_groups, and each_batch re-ran that CTE in its first boundary query, in the OFFSET 1000 boundary, and in the id pluck of every batch. The task no longer uses that CTE. Under limited indexing it now walks ElasticsearchIndexedNamespace.each_batch, loads the groups among each batch of 1,000 indexed namespaces, and batches each group's subtree with group.self_and_descendants.each_batch, a traversal_ids @> filter like the one the transfer service uses. Each subtree batch's ids are plucked, so every batch stays at most 1,000 ids, and the milestones are read with the same keyset iterator. Without limited indexing it batches over Group.all.
milestones = ::Milestone.of_groups(group_batch.pluck_primary_key).with_group.order_by_group_and_id
::Gitlab::Pagination::Keyset::Iterator.new(scope: milestones).each_batch do |batch|
::Elastic::ProcessInitialBookkeepingService.track!(*batch)
endOrdering by group_id, id lets index_milestones_on_group_id supply the order, so no keyset plan walks milestones_pkey. each_batch always orders by id. order_hint only adds group_id as a second sort key, so the planner can still walk the pkey when it overestimates how many milestones match. Each keyset page is one query. A page that starts inside a group reads all of that group's rows and sorts them: about 2.4k buffers per page for group 8985502, the group with the most milestones. A (group_id, id) index would avoid that, but it isn't worth an extra index on milestones for a background task.
Every keyset plan is in the table below.
Comparison with each_batch(order_hint:), and SQL
Every query each version sends for one outer batch, with real batch boundaries and cursors. The with_group preload is the same in both, so it isn't counted. Most runs were on a warm cache, so compare buffers.
| Case | Milestones | each_batch(order_hint:) |
Keyset |
|---|---|---|---|
gitlab-org, all 1,606 groups in one subquery |
260 | 3 queries, ~89k buffers (165505, 165593, 165594) | 2 queries, 11.3k buffers (165553, 165592) |
Real gitlab-org transfer, 2 outer batches |
260 | 6 queries, 24.7k buffers (165519–165524) | 4 queries, 16.1k buffers (165596, 165597, 165600, 165603) |
| Group 8985502, one group with 9 pages | 8,091 | 19 queries, ~43k buffers, extrapolated from one page (165506, 165507, 165528) | 10 queries, ~24k buffers, extrapolated from one page (165554, 165561, 165611) |
| Group 5518815, 999 groups | 0 | 1 query, 4,001 buffers (165544) | 1 query, 4,001 buffers (165604) |
These runs used the namespace subquery that both callers passed at the time. The plucked-id shape that both callers use now is cheaper: 377 buffers per next page, against 5,400 with the subquery (C9jG, HJBa).
The keyset version needs about half the queries, because each_batch sends a boundary probe and a separate load for every page. Each keyset page takes 3–4 ms longer to plan, because group_id is nullable and the iterator builds a 4-way UNION ALL. The null branches have a constant-false filter, so they read nothing.
The query from @johnmason's earlier review walks milestones_pkey with or without order_hint. It covers the whole gitlab-org tree in one namespace subquery: 82,720 buffers and 3.0 s cold without the hint (165549), 77,345 buffers and 5.35 s cold with it (165505). In the review run, the version with the hint got a sort that read 6,551 buffers.
-- id pluck, run once per group batch
SELECT "namespaces"."id" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND (traversal_ids @> ('{9970}')) AND "namespaces"."id" >= 9970 AND "namespaces"."id" < 99870106;
-- first page
SELECT "milestones".* FROM "milestones"
WHERE "milestones"."group_id" IN (<up to 1,000 plucked group ids>)
ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC LIMIT 1000;
-- next page, after cursor (group_id, id) = (<g>, <i>); <ids> are the plucked ids above
SELECT "milestones".* FROM (
(SELECT "milestones".* FROM "milestones" WHERE "milestones"."group_id" IN (<ids>) AND <g> IS NULL AND "milestones"."group_id" IS NULL AND "milestones"."id" > <i> ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC)
UNION ALL
(SELECT "milestones".* FROM "milestones" WHERE "milestones"."group_id" IN (<ids>) AND <g> IS NOT NULL AND "milestones"."group_id" IS NULL ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC)
UNION ALL
(SELECT "milestones".* FROM "milestones" WHERE "milestones"."group_id" IN (<ids>) AND <g> IS NOT NULL AND "milestones"."group_id" > <g> ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC)
UNION ALL
(SELECT "milestones".* FROM "milestones" WHERE "milestones"."group_id" IN (<ids>) AND <g> IS NOT NULL AND "milestones"."group_id" = <g> AND "milestones"."id" > <i> ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC)
) milestones
ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC LIMIT 1000;2. Group archive and unarchive
Search::Elastic::GroupArchivedEventWorker#track_group_milestones runs per batch of up to 500 namespace ids. It loads only the id and traversal_ids of those groups and keeps the ones that use Elasticsearch. It returns early if none are left. It then reads their milestones with Gitlab::Pagination::Keyset::Iterator over Milestone.of_groups(ids).with_group.order_by_group_and_id, the same as the transfer service and the rake task. The worker also defers on database health signals for the milestones table.
At the current HEAD, a batch of 500 gitlab-org group ids costs three queries: the group load (500 rows, 1,942 buffers, 824 ms cold, 166135), the first milestone page (257 rows, 888 buffers, 446 ms cold, 166136), and the empty final page that ends the iterator (97 buffers, 166137). The first page reads index_milestones_on_group_id and uses an incremental sort on id, so it doesn't walk milestones_pkey.
Measurements and SQL
An earlier version wrapped the load in each_batch. The reviewer measured that probe at about 19.9 GiB of buffers (pkey walk). A plain of_groups(ids) load read 30 buffers in 14 ms on production data, before the group pre-filter was added.
SELECT "namespaces"."id", "namespaces"."traversal_ids" FROM "namespaces" WHERE "namespaces"."type" = 'Group' AND "namespaces"."id" IN (<up to 500 ids>);
-- first page
SELECT "milestones".* FROM "milestones" WHERE "milestones"."group_id" IN (<indexed group ids>)
ORDER BY "milestones"."group_id" ASC, "milestones"."id" ASC LIMIT 1000;
-- next and final pages: the same 4-way UNION ALL as the transfer path, after cursor (group_id, id)
-- plus the with_group preload of namespaces by idPlans at HEAD, for 500 gitlab-org group ids. The final page uses cursor (58706865, 2823764), the last group in the batch with milestones visible to me.
- Group load: 166135
- First milestone page: 166136
- Final page (empty): 166137
with_grouppreload (namespaces.*by id, only for the groups in the page; 500 ids is the upper bound): 165509preload_for_indexinggroup settings: 165510 (namespace_settings), 165511 (namespace_settings_with_ancestors_inherited_settings)
3. Association indexer
Elastic::ProcessBookkeepingService.maintain_indexed_namespace_associations! now includes milestones for group namespaces. ElasticAssociationIndexerWorker then runs group.milestones.with_group.find_in_batches. It runs on visibility change, when a namespace is added to limited indexing, and on parent change. The with_group preload was added to fix an N+1.
This is the same WHERE group_id = ? ORDER BY id LIMIT shape that can make the planner walk milestones_pkey. At the current HEAD it doesn't, even for the group with the most milestones on GitLab.com: the planner reads the group's rows through index_milestones_on_group_id and sorts them.
| Group | Rows | Buffers | Time (cold) |
|---|---|---|---|
gitlab-org/search-team (no milestones) |
0 | 6 | 3.3 ms (plan) |
gitlab-org |
162 | 161 | 154 ms (plan) |
| Group 8985502 (most group milestones, 8,091) | 1,000 of 8,091 | 2,184 | 1.1 s, almost all disk reads (plan) |
For that group, find_in_batches runs 9 batches, and each one reads all of the group's rows before sorting. That's about 20k buffers in total, mostly from cache after the first batch. A (group_id, id) index would avoid this, but it doesn't seem worth it for a background worker that only runs on visibility change, limited-indexing changes, and parent change.
SQL
SELECT "milestones".* FROM "milestones" WHERE "milestones"."group_id" = 9970 ORDER BY "milestones"."id" ASC LIMIT 1000;Plans at HEAD: 164957 (gitlab-org/search-team), 164958 (gitlab-org), 165503 (group 8985502)
All query plans at HEAD (ae3ca02c)
Each query that this MR adds, from gitlab-production-main on postgres.ai. The keyset queries use real batch boundaries and cursors from production data. Many ran on a warm cache, so compare buffers.
| Code path | Query | Rows | Buffers | Time | Plan |
|---|---|---|---|---|---|
index_group_entities rake task, limited indexing |
Indexed namespace boundary, OFFSET 1000 |
– | 264 | 1 ms (warm) | 166712 |
| same | Group load for 1,000 indexed namespaces (approximated with a subquery instead of literal ids) | 661 | 5,264 | 1.5 s (cold) | 166714 |
Transfer and index_group_entities subtree walk (gitlab-org) |
Subtree boundary, OFFSET 1000 |
– | 1,611 | 1.6 s (cold) | 166708 |
| same | Subtree id pluck | – | 1,608 | 8 ms (cache warmed by a concurrent query) | 166709 |
| same | Milestone first page, 1,000 literal ids | 259 | 1,103 | 765 ms (mostly cold) | 166710 |
| same | Milestone next page, 1,000 literal ids | 2 | 377 | 0.6 ms | C9jG (from the DB review) |
| same | Milestone final page (empty) | 0 | 19 | 24 ms | 166711 |
Archive worker, 500 gitlab-org ids |
group load (id, traversal_ids) |
500 | 1,942 | 824 ms | 166135 |
Archive worker, 500 gitlab-org ids |
first milestone page | 257 | 888 | 446 ms | 166136 |
Archive worker, 500 gitlab-org ids |
final page (empty) | 0 | 97 | 20 ms | 166137 |
Archive worker, 500 gitlab-org ids |
with_group preload, upper bound (namespaces.* for 500 ids) |
500 | 1,942 | 858 ms | 165509 |
Association indexer, gitlab-org/search-team |
find_in_batches batch |
0 | 6 | 3.3 ms | 164957 |
Association indexer, gitlab-org |
find_in_batches batch |
162 | 161 | 154 ms | 164958 |
| Association indexer, group 8985502 | find_in_batches batch |
1,000 | 2,184 | 1.1 s | 165503 |
preload_for_indexing, 500 group ids |
namespace_settings |
500 | 1,912 | 506 ms | 165510 |
preload_for_indexing, 500 group ids |
namespace_settings_with_ancestors_inherited_settings |
500 | 18,798 | 555 ms | 165511 |
The subtree rows apply to both the transfer service and the rake task, since both now pluck ids. The earlier transfer rows measured the subquery shape and were removed. Group 8985502 (8,091 milestones in one group) takes 9 pages, and each page reads and sorts all of the group's rows: about 2.4k buffers (165561, measured with the subquery, which is cheap for a single group). Times mix cold and warm runs, so compare buffers.
The database review at 08b242fa found nothing blocking, and its one improvement is applied. Ready for database maintainer review.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.