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, and archived (build_group_data in ee/lib/elastic/latest/milestone_instance_proxy.rb). namespace_id is 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), using group.traversal_ids.first to avoid a root_ancestor query per group. Project milestones keep project_<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 from es_parent. Removing join_field from the mapping is tracked in #630682. There's no SCHEMA_VERSION bump: project milestone documents are unchanged, and group milestone documents are new, so no migration needs to find old versions.
  • Milestone.preload_for_indexing preloads the group's namespace settings, so reading archived doesn't cost a query per milestone. The does not have N+1 queries for group milestones spec in ee/spec/services/elastic/process_bookkeeping_service_spec.rb covers this: without the group: preload it goes from 5 to 11 queries.
  • Milestone#use_elasticsearch? (EE) now returns project.use_elasticsearch? for project milestones and group.use_elasticsearch? for group milestones. It used to fall back to project&.use_elasticsearch?, which is nil for 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::ElasticGroupAssociationDeletionWorker now also runs a delete_by_query on the main index for group milestone documents, routed by n_<ancestor_id> and filtered on type: milestone, namespace_id in the group ids, and a traversal_ids prefix <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_id is ON 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 includes milestones by default for group namespaces, like epics. This makes the existing callers reindex group milestones: the visibility change callback in Elastic::MaintainElasticsearchOnGroupUpdate, ElasticNamespaceIndexerWorker when a namespace is added to limited indexing, and Elastic::NamespaceUpdateWorker on parent change.
    • Transfer: EE::Groups::TransferService#process_group_associations reindexes the group milestones of the transferred group and its descendants when the group uses Elasticsearch, so they pick up the new traversal_ids and routing. Wikis, epics, and milestones now share a single group.use_elasticsearch? check there. process_wikis moved inside it from post_update_hooks and lost its own check. This doesn't change behaviour, because Group#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 (handles Namespaces::Groups::GroupArchivedEvent, published by both services) also tracks the group milestones of the group and its descendants that use Elasticsearch.
    • Rake tasks: gitlab:elastic:index already covers group milestones through ElasticNamespaceIndexerWorker, which calls maintain_indexed_namespace_associations!. gitlab:elastic:index_group_entities now 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), alongside of_projects.
  • 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

Screenshots or screen recordings

No UI change. This MR is backend-only.

How to set up and validate locally

  1. Enable Advanced Search indexing in GDK.
  2. Create a group milestone and check its document: _routing is n_<root group id>, and it has namespace_id, traversal_ids, visibility_level, and archived.
  3. Archive the group and check archived becomes true on the document after indexing. Change the group's visibility and check visibility_level updates.
  4. Transfer a subgroup that has a milestone to another root and check that exactly one document remains, routed by the new root.
  5. Delete the group and check the document is gone.
  6. 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.
  7. 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)
end

Ordering 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 id

Plans 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_group preload (namespaces.* by id, only for the groups in the page; 500 ids is the upper bound): 165509
  • preload_for_indexing group 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.

🤖 Generated with Claude Code

Edited by Ravi Kumar

Merge request reports

Loading
Loading