Skip to content

Commit 531cba7

Browse files
author
GitLab Bot
committed
Add latest changes from gitlab-org/gitlab@master
1 parent da2739f commit 531cba7

16 files changed

+328
-42
lines changed

app/services/environments/delete_managed_resources_service.rb

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@ module Environments
44
class DeleteManagedResourcesService
55
include Gitlab::Utils::StrongMemoize
66

7-
def initialize(environment)
7+
def initialize(environment, current_user:)
88
@environment = environment
9+
@current_user = current_user
910
end
1011

1112
def execute
@@ -15,12 +16,14 @@ def execute
1516

1617
Clusters::Agents::ManagedResources::DeleteWorker.perform_async(managed_resource.id)
1718

19+
emit_event
20+
1821
ServiceResponse.success
1922
end
2023

2124
private
2225

23-
attr_reader :environment
26+
attr_reader :environment, :current_user
2427

2528
def can_delete_resources?
2629
environment.stopped? &&
@@ -33,5 +36,18 @@ def managed_resource
3336
environment.managed_resources.completed.order_id_desc.first
3437
end
3538
strong_memoize_attr :managed_resource
39+
40+
def emit_event
41+
Gitlab::InternalEvents.track_event(
42+
'delete_environment_for_managed_resource',
43+
user: current_user,
44+
project: environment.project,
45+
additional_properties: {
46+
label: environment.project.namespace.actual_plan_name,
47+
property: environment.tier,
48+
value: environment.id
49+
}
50+
)
51+
end
3652
end
3753
end

app/services/environments/stop_service.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def environments
7878
end
7979

8080
def delete_managed_resources(environment)
81-
Environments::DeleteManagedResourcesService.new(environment).execute
81+
Environments::DeleteManagedResourcesService.new(environment, current_user:).execute
8282
end
8383
end
8484
end
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
description: Tracks deleted managed resource
3+
internal_events: true
4+
action: delete_environment_for_managed_resource
5+
identifiers:
6+
- project
7+
- namespace
8+
- user
9+
additional_properties:
10+
label:
11+
description: pricing tier
12+
property:
13+
description: environment tier
14+
value:
15+
description: environment id
16+
product_group: environments
17+
product_categories:
18+
- environment_management
19+
milestone: '18.0'
20+
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188965
21+
tiers:
22+
- premium
23+
- ultimate
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
key_path: redis_hll_counters.count_distinct_user_id_from_delete_environment_for_managed_resource
3+
description: Count of unique users who deleted the environment using managed resource
4+
product_group: environments
5+
product_categories:
6+
- environment_management
7+
performance_indicator_type: []
8+
value_type: number
9+
status: active
10+
milestone: '18.0'
11+
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188965
12+
time_frame:
13+
- 28d
14+
- 7d
15+
data_source: internal_events
16+
data_category: optional
17+
tiers:
18+
- premium
19+
- ultimate
20+
events:
21+
- name: delete_environment_for_managed_resource
22+
unique: user.id
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
---
2+
key_path: redis_hll_counters.count_distinct_value_from_delete_environment_for_managed_resource
3+
description: Count of unique environments that deleted environments using managed resource
4+
product_group: environments
5+
product_categories:
6+
- environment_management
7+
performance_indicator_type: []
8+
value_type: number
9+
status: active
10+
milestone: '18.0'
11+
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/188965
12+
time_frame:
13+
- 28d
14+
- 7d
15+
data_source: internal_events
16+
data_category: optional
17+
tiers:
18+
- premium
19+
- ultimate
20+
events:
21+
- name: delete_environment_for_managed_resource
22+
unique: value
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
# frozen_string_literal: true
2+
3+
class RePrepareAsyncIndexOnMergeRequestCommitsMetadataId < Gitlab::Database::Migration[2.3]
4+
milestone '18.0'
5+
6+
INDEX_NAME = 'index_mrdc_on_merge_request_commits_metadata_id'
7+
8+
# TODO: Index to be created synchronously in https://gitlab.com/gitlab-org/gitlab/-/issues/527227
9+
# rubocop:disable Migration/PreventIndexCreation -- this index is required as
10+
# we will be querying data from `merge_request_commits_metadata_id` and joining
11+
# by this column.
12+
def up
13+
prepare_async_index :merge_request_diff_commits, :merge_request_commits_metadata_id, name: INDEX_NAME,
14+
where: "merge_request_commits_metadata_id IS NOT NULL"
15+
end
16+
# rubocop:enable Migration/PreventIndexCreation
17+
18+
def down
19+
unprepare_async_index :merge_request_diff_commits, :merge_request_commits_metadata_id, name: INDEX_NAME
20+
end
21+
end

db/schema_migrations/20250430105707

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
06daee445c8a41f7d539e05e6f75ff810b387185491d15362bdaa51a70e6bcbe

doc/.vale/gitlab_base/SubstitutionWarning.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ swap:
4545
log-in: "sign in"
4646
logged in user: "authenticated user"
4747
logged-in user: "authenticated user"
48+
lower case: "lowercase"
49+
lower-case: "lowercase"
4850
machine-learning: "machine learning"
4951
modal dialog: "dialog"
5052
modal window: "dialog"
@@ -74,6 +76,10 @@ swap:
7476
since: "because' or 'after"
7577
source (?:install|installation): self-compiled installation
7678
source (?:installs|installations): self-compiled installations
79+
sub directories: "subdirectories"
80+
sub-directories: "subdirectories"
81+
sub directory: "subdirectory"
82+
sub-directory: "subdirectory"
7783
sub group: "subgroup"
7884
sub-group: "subgroup"
7985
sub-groups: "subgroups"

doc/development/database/adding_database_indexes.md

Lines changed: 134 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -245,9 +245,21 @@ for new and updated records while the removal and fix are in progress.
245245
The details of the work might vary and require different approaches.
246246
Consult the Database team, reviewers, or maintainers to plan the work.
247247

248-
## Finding Unused Indexes
248+
## Dropping unused indexes
249249

250-
To see which indexes are unused you can run the following query:
250+
Unused indexes should be dropped because they increase [maintainence overhead](#maintenance-overhead), consume
251+
disk space, and can degrade query planning efficiency without providing any performance benefit.
252+
However, dropping an index that's still used could result in query performance degradation or timeouts,
253+
potentially leading to incidents. It's important to [verify the index is unused](#verifying-that-an-index-is-unused)
254+
on both on GitLab.com and self-managed instances prior to removal.
255+
256+
- For large tables, consider [dropping the index asynchronously](#drop-indexes-asynchronously).
257+
- For partitioned tables, only the parent index can be dropped. PostgreSQL does not permit child indexes
258+
(i.e. the corresponding indexes on its partitions) to be independently removed.
259+
260+
### Finding possible unused indexes
261+
262+
To see which indexes are candidates for removal, you can run the following query:
251263

252264
```sql
253265
SELECT relname as table_name, indexrelname as index_name, idx_scan, idx_tup_read, idx_tup_fetch, pg_size_pretty(pg_relation_size(indexrelname::regclass))
@@ -259,28 +271,132 @@ AND idx_tup_fetch = 0
259271
ORDER BY pg_relation_size(indexrelname::regclass) desc;
260272
```
261273

262-
This query outputs a list containing all indexes that are never used and sorts
263-
them by indexes sizes in descending order. This query helps in
264-
determining whether existing indexes are still required. More information on
265-
the meaning of the various columns can be found at
274+
This query outputs a list containing all indexes that have not been used since the stats were last reset and sorts
275+
them by index size in descending order. More information on the meaning of the various columns can be found at
266276
<https://www.postgresql.org/docs/current/monitoring-stats.html>.
267277

268-
To determine if an index is still being used on production, use [Grafana](https://dashboards.gitlab.net/explore?schemaVersion=1&panes=%7B%22pum%22%3A%7B%22datasource%22%3A%22mimir-gitlab-gprd%22%2C%22queries%22%3A%5B%7B%22refId%22%3A%22A%22%2C%22expr%22%3A%22sum+by+%28type%29%28rate%28pg_stat_user_indexes_idx_scan%7Benv%3D%5C%22gprd%5C%22%2C+indexrelname%3D%5C%22INSERT+INDEX+NAME+HERE%5C%22%7D%5B30d%5D%29%29%22%2C%22range%22%3Atrue%2C%22instant%22%3Atrue%2C%22datasource%22%3A%7B%22type%22%3A%22prometheus%22%2C%22uid%22%3A%22mimir-gitlab-gprd%22%7D%2C%22editorMode%22%3A%22code%22%2C%22legendFormat%22%3A%22__auto%22%7D%5D%2C%22range%22%3A%7B%22from%22%3A%22now-1h%22%2C%22to%22%3A%22now%22%7D%7D%7D&orgId=1):
278+
For GitLab.com, you can check the latest generated [production reports](https://console.postgres.ai/gitlab/reports/)
279+
on postgres.ai and inspect the `H002 Unused Indexes` file.
269280

270-
```sql
271-
sum by (type)(rate(pg_stat_user_indexes_idx_scan{env="gprd", indexrelname="INSERT INDEX NAME HERE"}[30d]))
272-
```
281+
{{< alert type="warning" >}}
273282

274-
Because the query output relies on the actual usage of your database, it
275-
may be affected by factors such as:
283+
These reports only show indexes that have no recorded usage **since the last statistics reset.**
284+
They do not guarantee that the indexes are never used.
276285

277-
- Certain queries never being executed, thus not being able to use certain
278-
indexes.
279-
- Certain tables having little data, resulting in PostgreSQL using sequence
280-
scans instead of index scans.
286+
{{< /alert >}}
281287

282-
This data is only reliable for a frequently used database with
283-
plenty of data, and using as many GitLab features as possible.
288+
### Verifying that an index is unused
289+
290+
This section contains resources to help you evaluate an index and confirm that it's safe to remove. Note that
291+
this is only a suggested guide and is not exhaustive. Ultimately, the goal is to gather enough data to justify
292+
dropping the index.
293+
294+
Be aware that certain factors can give the false impression that an index is unused, such as:
295+
296+
- There may be queries that run on self-managed but not on GitLab.com.
297+
- The index may be used for very infrequent processes such as periodic cron jobs.
298+
- On tables that have little data, PostgreSQL may initially prefer a sequential scan over an index scan
299+
until the table is large enough.
300+
301+
#### Investigating index usage
302+
303+
1. Start by gathering all the metadata available for the index, verifying its name and definition.
304+
- The index name in the development environment may not match production. It's important to correlate the indexes
305+
based on definition rather than name. To check its definition, you can:
306+
- Manually inspect [db/structure.sql](https://gitlab.com/gitlab-org/gitlab/-/blob/master/db/structure.sql)
307+
(This file does **not** include data on dynamically generated partitions.)
308+
- [Use Database Lab to check the status of an index.](database_lab.md#checking-indexes)
309+
- For partitioned tables, child indexes are often named differently than the parent index.
310+
To list all child indexes, you can:
311+
- Run `\d+ <PARENT_INDEX_NAME>` in [Database Lab](database_lab.md).
312+
- Run the following query to see the full parent-child index structure in more detail:
313+
314+
```sql
315+
SELECT
316+
parent_idx.relname AS parent_index,
317+
child_tbl.relname AS child_table,
318+
child_idx.relname AS child_index,
319+
dep.deptype,
320+
pg_get_indexdef(child_idx.oid) AS child_index_def
321+
FROM
322+
pg_class parent_idx
323+
JOIN pg_depend dep ON dep.refobjid = parent_idx.oid
324+
JOIN pg_class child_idx ON child_idx.oid = dep.objid
325+
JOIN pg_index i ON i.indexrelid = child_idx.oid
326+
JOIN pg_class child_tbl ON i.indrelid = child_tbl.oid
327+
WHERE
328+
parent_idx.relname = '<PARENT_INDEX_NAME>';
329+
```
330+
331+
- Run the following command in the Rails console:
332+
333+
```ruby
334+
Gitlab::Database::PostgresPartitionedTable.by_identifier('public.<PARENT_TABLE_NAME>').indexes
335+
```
336+
337+
1. For GitLab.com, you can view index usage data in [Grafana](https://dashboards.gitlab.net/goto/shHCmIxHg?orgId=1).
338+
- Query the metric `pg_stat_user_indexes_idx_scan` filtered by the relevant index(s) for at least the last 6 months.
339+
The query below shows index usage across all database instances combined.
340+
341+
```sql
342+
sum by (indexrelname) (pg_stat_user_indexes_idx_scan{env="gprd", relname=~"<TABLE_NAME_REGEX>", indexrelname=~"<INDEX_NAME_REGEX>"})
343+
```
344+
345+
- For partitioned tables, we must check that **all child indexes are unused** prior to dropping the parent.
346+
347+
If the data shows that an index has zero or negligible usage, it's a strong candidate for removal. However, keep in mind that
348+
this is limited to usage on GitLab.com. We should still [investigate all related queries](#investigating-related-queries) to
349+
ensure it can be safely removed for self-managed instances.
350+
351+
An index that shows low usage may still be dropped **if** we can confirm that other existing indexes would sufficiently
352+
support the queries using it. PostgreSQL decides which index to use based on data distribution statistics, so in certain
353+
situations it may slightly prefer one index over another even if both indexes adequately support the query, which may
354+
account for the occasional usage.
355+
356+
#### Investigating related queries
357+
358+
The following are ways to find all queries that _may_ utilize the index. It's important to understand the context in
359+
which the queries are or may be executed so that we can determine if the index either:
360+
361+
- Has no queries on GitLab.com nor on self-managed that depend on it.
362+
- Can be sufficiently supported by other existing indexes.
363+
364+
1. Investigate the origins of the index.
365+
- Dig through the commit history, related merge requests, and issues that introduced the index.
366+
- Try to find answers to questions such as:
367+
- Why was the index added in the first place? What query was it meant to support?
368+
- Does that query still exist and get executed?
369+
- Is it only applicable to self-managed instances?
370+
371+
1. Examine queries outputted from running the [`rspec:merge-auto-explain-logs`](https://gitlab.com/gitlab-org/gitlab/-/jobs/9805995367) CI job.
372+
- This job collects and analyzes queries executed through tests. The output is saved as an artifact: `auto_explain/auto_explain.ndjson.gz`
373+
- Since we don't always have 100% test coverage, this job may not capture all possible queries and variations.
374+
375+
1. Examine queries recorded in [postgres logs](https://log.gprd.gitlab.net/app/r/s/A55hK) on Kibana.
376+
- Generally, you can filter for `json.sql` values that contain the table name and at least one of the columns from the index definition
377+
(including conditions). Example KQL:
378+
379+
```plaintext
380+
json.sql: <TABLE_NAME> AND (json.sql: *<COLUMN_NAME_1>* OR json.sql: *<COLUMN_NAME_2>*)
381+
```
382+
383+
- While there are many factors that affect index usage, the query's filtering and ordering clauses often have the most influence.
384+
So focus on finding queries with relevant columns in the predicate/conditions rather than in the `SELECT` clause.
385+
- Caveat: We only keep the last 7 days of logs and this data does not apply to self-managed usage.
386+
387+
1. Manually search through the GitLab codebase.
388+
- This process may be tedious but it's the most reliable way to ensure there are no other queries we missed from the previous actions,
389+
especially ones that are infrequent or only apply to self-managed instances.
390+
- It's possible there are queries that were introduced some time after the index was initially added,
391+
so we can't always depend on the index origins; we must also examine the current state of the codebase.
392+
- To help direct your search, try to gather context about how the table is used and what features access it. Look for queries
393+
that involve any of the columns from the index definition, particularly those that are part of the filtering or ordering clauses.
394+
- Another approach is to conduct a keyword search for the model/table name and any relevant columns. However, this could be a
395+
trickier and long-winded process since some queries may be dynamically compiled from code across multiple files.
396+
397+
After collecting the relevant queries, you can then obtain [EXPLAIN plans](understanding_explain_plans.md) to help you assess if a query
398+
relies on the index in question. For this process, it's necessary to have a good understanding of how indexes support queries and how
399+
their usage is affected by data distribution changes. We recommend seeking guidance from a database domain expert to help with your assessment.
284400

285401
## Requirements for naming indexes
286402

lib/gitlab/auth/oidc/step_up_authentication.rb

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ module Oidc
88
# This module manages the configuration and validation of step-up authentication
99
# requirements for OAuth providers, particularly focusing on admin mode access.
1010
module StepUpAuthentication
11+
SESSION_STORE_KEY = 'omniauth_step_up_auth'
12+
1113
STEP_UP_AUTH_SCOPE_ADMIN_MODE = :admin_mode
1214

1315
class << self
@@ -36,17 +38,13 @@ def enabled_for_provider?(provider_name:, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE)
3638
# @return [Boolean] true if step-up authentication is authenticated
3739
def succeeded?(session, scope: STEP_UP_AUTH_SCOPE_ADMIN_MODE)
3840
step_up_auth_flows =
39-
session&.dig('omniauth_step_up_auth')
40-
.to_h
41-
.flat_map do |provider, step_up_auth_object|
42-
step_up_auth_object.map do |step_up_auth_scope, _|
43-
::Gitlab::Auth::Oidc::StepUpAuthenticationFlow.new(
44-
session: session,
45-
provider: provider,
46-
scope: step_up_auth_scope
47-
)
41+
omniauth_step_up_auth_session_data(session)
42+
&.to_h
43+
&.flat_map do |provider, step_up_auth_object|
44+
step_up_auth_object.map do |step_up_auth_scope, _|
45+
build_flow(provider: provider, session: session, scope: step_up_auth_scope)
46+
end
4847
end
49-
end
5048
step_up_auth_flows
5149
.select do |step_up_auth_flow|
5250
step_up_auth_flow.scope.to_s == scope.to_s
@@ -79,6 +77,10 @@ def slice_relevant_id_token_claims(oauth_raw_info:, provider:, scope: STEP_UP_AU
7977
oauth_raw_info.slice(*relevant_id_token_claims)
8078
end
8179

80+
def omniauth_step_up_auth_session_data(session)
81+
Gitlab::NamespacedSessionStore.new(SESSION_STORE_KEY, session)
82+
end
83+
8284
private
8385

8486
def oauth_providers

0 commit comments

Comments
 (0)