Repository: mlflow/mlflow

Issue:
[FR] Extend `mlflow gc` to support removing deleted experiments
### Willingness to contribute

No. I cannot contribute this feature at this time.

### Proposal Summary

Add an `--experiment-ids` flag to the `mlflow gc` CLI (https://mlflow.org/docs/latest/cli.html#mlflow-gc). For each experiment ID that is specified, `mlflow gc` should remove the experiment and all of its runs. 

Before performing deletion, `mlflow gc` should verify that all of the specified experiments are in the `DELETED` state. `mlflow gc` should support specifying `--experiment-ids` and `--run-ids` at the same time, and `--older-than` should be extended to apply to runs and experiments

### Motivation

> #### What is the use case for this feature?

Enable MLflow admins to permanently remove deleted experiments

> #### Why is this use case valuable to support for MLflow users in general?

The inability to delete experiments means that experiment names cannot be reused and experiment storage space cannot be freed. This FR would address these issues.

> #### Why is this use case valuable to support for your project(s) or organization?

^

> #### Why is it currently difficult to achieve this use case?

MLflow does not currently offer a way to permanently remove deleted experiments

### Details

_No response_

### What component(s) does this bug affect?

- [ ] `area/artifacts`: Artifact stores and artifact logging
- [ ] `area/build`: Build and test infrastructure for MLflow
- [ ] `area/docs`: MLflow documentation pages
- [ ] `area/examples`: Example code
- [ ] `area/model-registry`: Model Registry service, APIs, and the fluent client calls for Model Registry
- [ ] `area/models`: MLmodel format, model serialization/deserialization, flavors
- [ ] `area/pipelines`: Pipelines, Pipeline APIs, Pipeline configs, Pipeline Templates
- [ ] `area/projects`: MLproject format, project running backends
- [ ] `area/scoring`: MLflow Model server, model deployment tools, Spark UDFs
- [ ] `area/server-infra`: MLflow Tracking server backend
- [X] `area/tracking`: Tracking Service, tracking client APIs, autologging

### What interface(s) does this bug affect?

- [ ] `area/uiux`: Front-end, user experience, plotting, JavaScript, JavaScript dev server
- [ ] `area/docker`: Docker use across MLflow's components, such as MLflow Projects and MLflow Models
- [ ] `area/sqlalchemy`: Use of SQLAlchemy in the Tracking Service or Model Registry
- [ ] `area/windows`: Windows support

### What language(s) does this bug affect?

- [ ] `language/r`: R APIs and clients
- [ ] `language/java`: Java APIs and clients
- [ ] `language/new`: Proposals for new client languages

### What integration(s) does this bug affect?

- [ ] `integrations/azure`: Azure and Azure ML integrations
- [ ] `integrations/sagemaker`: SageMaker integrations
- [ ] `integrations/databricks`: Databricks integrations

Discussion:
harupy: @shaikmoeed We have tests for `mlflow gc` in this file:

https://github.com/mlflow/mlflow/blob/b26eb133f96fd0cfd4833546490730367ccef46f/tests/test_cli.py#L259-L270

Can you add tests for `--experiemnt_ids`?
shaikmoeed: 
> Can you add tests for `--experiemnt_ids`?

@harupy I am a little new to writing test cases, I can write with your guidance?
What test cases need to cover?
harupy: > I am a little new to writing test cases, I can write with your guidance?

Got it. Can I push a commit to add one test case? I think that would help understand how to test.
shaikmoeed: > Got it. Can I push a commit to add one test case? I think that would help you understand how to test.

@harupy  Sure, that helps to get started!

harupy: @shaikmoeed Good question Mine looks like this:

```json
{
  "python.defaultInterpreterPath": "/home/haru/miniconda3/envs/mlflow-dev-env/bin/python",
  "python.formatting.provider": "black",
  "cSpell.enabled": true,
  "liveServer.settings.port": 5501,
  "prettier.configPath": "mlflow/server/js/.prettierrc.js",
  "python.testing.pytestArgs": ["tests"],
  "python.testing.unittestEnabled": false,
  "python.testing.pytestEnabled": true
}
```
shaikmoeed: @harupy I have a doubt about pylint settings used in mlflow. It's giving 10/10 for the below code which is not following snake_case for functions.
```
def AbcDef():
    return "some"
```

Is there any reason to ignore this validation check or is it fine to have such names?
harupy: > Is there any reason to ignore this validation check or is it fine to have such names? If I want to enable this check for my own development work, where can I do it?

No, it's not a good practice to camel-casing for function names. PyLint C103 should prevent this, but it's disabled now.
shaikmoeed: @harupy Is there a way to re-run failed check. 

Error:

> requests.exceptions.ConnectionError: HTTPSConnectionPool(host='pypi.python.org', port=443): Max retries exceeded with url: /pypi/scikit-learn/json (Caused by NewConnectionError('<urllib3.connection.HTTPSConnection object at 0x7fa445142550>: Failed to establish a new connection: [Errno 101] Network is unreachable'))

Patch:
diff --git a/mlflow/cli.py b/mlflow/cli.py
index 5c6e5e3e2..845069f01 100644
--- a/mlflow/cli.py
+++ b/mlflow/cli.py
@@ -9,6 +9,7 @@ from click import UsageError
 from datetime import timedelta
 
 import mlflow.db
+from mlflow.entities import ViewType
 import mlflow.experiments
 import mlflow.deployments.cli
 from mlflow import projects
@@ -531,17 +532,37 @@ def server(
     " are not specified, data is removed for all runs in the `deleted`"
     " lifecycle stage.",
 )
-def gc(older_than, backend_store_uri, run_ids):
+@click.option(
+    "--experiment-ids",
+    default=None,
+    help="Optional comma separated list of experiments to be permanently deleted including "
+    "all of their associated runs. If experiment ids are not specified, data is removed for all "
+    "experiments in the `deleted` lifecycle stage.",
+)
+def gc(older_than, backend_store_uri, run_ids, experiment_ids):
     """
     Permanently delete runs in the `deleted` lifecycle stage from the specified backend store.
     This command deletes all artifacts and metadata associated with the specified runs.
     """
+    import warnings
+    from mlflow.utils.time_utils import get_current_time_millis
+
     backend_store = _get_store(backend_store_uri, None)
+    skip_experiments = False
     if not hasattr(backend_store, "_hard_delete_run"):
         raise MlflowException(
             "This cli can only be used with a backend that allows hard-deleting runs"
         )
 
+    if not hasattr(backend_store, "_hard_delete_experiment"):
+        warnings.warn(
+            "The specified backend does not allow hard-deleting experiments. Experiments"
+            " will be skipped.",
+            FutureWarning,
+            stacklevel=2,
+        )
+        skip_experiments = True
+
     time_delta = 0
 
     if older_than is not None:
@@ -562,16 +583,53 @@ def gc(older_than, backend_store_uri, run_ids):
     deleted_run_ids_older_than = backend_store._get_deleted_runs(older_than=time_delta)
     if not run_ids:
         run_ids = deleted_run_ids_older_than
-
     else:
         run_ids = run_ids.split(",")
 
-    for run_id in run_ids:
+    if not skip_experiments:
+        deleted_older_experiment_ids = []
+        next_experiment_page_token = None
+        time_threshold = get_current_time_millis() - time_delta
+        filter_string = f"last_update_time < {time_threshold}" if older_than else ""
+        while True:
+            page_results = backend_store.search_experiments(
+                view_type=ViewType.DELETED_ONLY,
+                filter_string=filter_string,
+                page_token=next_experiment_page_token,
+            )
+            for experiment in page_results:
+                deleted_older_experiment_ids.append(experiment.experiment_id)
+            if page_results.token is None:
+                break
+            next_experiment_page_token = page_results.token
+
+        if experiment_ids:
+            experiment_ids = experiment_ids.split(",")
+        else:
+            experiment_ids = deleted_older_experiment_ids
+
+        next_run_page_token = None
+        while True:
+            page_results = backend_store.search_runs(
+                experiment_ids=experiment_ids,
+                filter_string="",
+                run_view_type=ViewType.DELETED_ONLY,
+                page_token=next_run_page_token,
+            )
+
+            for run in page_results:
+                run_ids.append(run.info.run_id)
+
+            if page_results.token is None:
+                break
+            next_run_page_token = page_results.token
+
+    for run_id in set(run_ids):
         run = backend_store.get_run(run_id)
         if run.info.lifecycle_stage != LifecycleStage.DELETED:
             raise MlflowException(
-                "Run % is not in `deleted` lifecycle stage. Only runs in "
-                "`deleted` lifecycle stage can be deleted." % run_id
+                "Run % is not in `deleted` lifecycle stage. Only runs in"
+                " `deleted` lifecycle stage can be deleted." % run_id
             )
         # raise MlflowException if run_id is newer than older_than parameter
         if older_than and run_id not in deleted_run_ids_older_than:
@@ -585,6 +643,23 @@ def gc(older_than, backend_store_uri, run_ids):
         backend_store._hard_delete_run(run_id)
         click.echo("Run with ID %s has been permanently deleted." % str(run_id))
 
+    if not skip_experiments:
+        invalid_experiments = []
+        for experiment_id in experiment_ids:
+            if older_than and experiment_id not in deleted_older_experiment_ids:
+                invalid_experiments.append(experiment_id)
+
+        if invalid_experiments:
+            raise MlflowException(
+                f"Experiments {invalid_experiments} are not older than the required age. "
+                f"Only runs older than {older_than} can be deleted.",
+                error_code=INVALID_PARAMETER_VALUE,
+            )
+
+        for experiment_id in experiment_ids:
+            backend_store._hard_delete_experiment(experiment_id)
+            click.echo("Experiment with ID %s has been permanently deleted." % str(experiment_id))
+
 
 cli.add_command(mlflow.deployments.cli.commands)
 cli.add_command(mlflow.experiments.commands)
diff --git a/mlflow/store/tracking/file_store.py b/mlflow/store/tracking/file_store.py
index d94f7b6a4..06667849e 100644
--- a/mlflow/store/tracking/file_store.py
+++ b/mlflow/store/tracking/file_store.py
@@ -454,6 +454,14 @@ class FileStore(AbstractStore):
         )
         mv(experiment_dir, self.trash_folder)
 
+    def _hard_delete_experiment(self, experiment_id):
+        """
+        Permanently delete an experiment.
+        This is used by the ``mlflow gc`` command line and is not intended to be used elsewhere.
+        """
+        experiment_dir = self._get_experiment_path(experiment_id, ViewType.DELETED_ONLY)
+        shutil.rmtree(experiment_dir)
+
     def restore_experiment(self, experiment_id):
         experiment_dir = self._get_experiment_path(experiment_id, ViewType.DELETED_ONLY)
         if experiment_dir is None:
diff --git a/mlflow/store/tracking/sqlalchemy_store.py b/mlflow/store/tracking/sqlalchemy_store.py
index 38cf6e14c..4b95f769c 100644
--- a/mlflow/store/tracking/sqlalchemy_store.py
+++ b/mlflow/store/tracking/sqlalchemy_store.py
@@ -487,6 +487,17 @@ class SqlAlchemyStore(AbstractStore):
                 self._mark_run_deleted(session, run)
             self._save_to_db(objs=experiment, session=session)
 
+    def _hard_delete_experiment(self, experiment_id):
+        """
+        Permanently delete a experiment (metadata and metrics, tags, parameters).
+        This is used by the ``mlflow gc`` command line and is not intended to be used elsewhere.
+        """
+        with self.ManagedSessionMaker() as session:
+            experiment = self._get_experiment(
+                experiment_id=experiment_id, session=session, view_type=ViewType.DELETED_ONLY
+            )
+            session.delete(experiment)
+
     def _mark_run_deleted(self, session, run):
         run.lifecycle_stage = LifecycleStage.DELETED
         run.deleted_time = get_current_time_millis()


Test patch:
diff --git a/tests/test_cli.py b/tests/test_cli.py
index e28eaca11..ec85ea36e 100644
--- a/tests/test_cli.py
+++ b/tests/test_cli.py
@@ -15,7 +15,7 @@ import numpy as np
 import pandas as pd
 
 import mlflow
-from mlflow.cli import server, ui
+from mlflow.cli import server, ui, gc
 from mlflow import pyfunc
 from mlflow.server import handlers
 from mlflow.store.tracking.sqlalchemy_store import SqlAlchemyStore
@@ -321,6 +321,59 @@ def test_mlflow_gc_file_store_older_than(file_store):
     assert len(runs) == 0
 
 
+@pytest.mark.parametrize("get_store_details", ["file_store", "sqlite_store"])
+def test_mlflow_gc_experiments(get_store_details, request):
+    def invoke_gc(*args):
+        return CliRunner().invoke(gc, args, catch_exceptions=False)
+
+    store, uri = request.getfixturevalue(get_store_details)
+    exp_id_1 = store.create_experiment("1")
+    run_id_1 = store.create_run(exp_id_1, user_id="user", start_time=0, tags=[], run_name="1")
+    invoke_gc("--backend-store-uri", uri)
+    experiments = store.search_experiments(view_type=ViewType.ALL)
+    exp_ids = [e.experiment_id for e in experiments]
+    runs = store.search_runs(experiment_ids=exp_ids, filter_string="", run_view_type=ViewType.ALL)
+    assert exp_ids == [exp_id_1, store.DEFAULT_EXPERIMENT_ID]
+    assert [r.info.run_id for r in runs] == [run_id_1.info.run_id]
+
+    store.delete_experiment(exp_id_1)
+    invoke_gc("--backend-store-uri", uri)
+    experiments = store.search_experiments(view_type=ViewType.ALL)
+    runs = store.search_runs(experiment_ids=exp_ids, filter_string="", run_view_type=ViewType.ALL)
+    assert [e.experiment_id for e in experiments] == [store.DEFAULT_EXPERIMENT_ID]
+    assert runs == []
+
+    exp_id_2 = store.create_experiment("2")
+    exp_id_3 = store.create_experiment("3")
+    store.delete_experiment(exp_id_2)
+    store.delete_experiment(exp_id_3)
+    invoke_gc("--backend-store-uri", uri, "--experiment-ids", exp_id_2)
+    experiments = store.search_experiments(view_type=ViewType.ALL)
+    assert [e.experiment_id for e in experiments] == [exp_id_3, store.DEFAULT_EXPERIMENT_ID]
+
+    with mock.patch("time.time", return_value=0) as mock_time:
+        exp_id_4 = store.create_experiment("4")
+        store.delete_experiment(exp_id_4)
+        mock_time.assert_called()
+
+    invoke_gc("--backend-store-uri", uri, "--older-than", "1d")
+    experiments = store.search_experiments(view_type=ViewType.ALL)
+    assert [e.experiment_id for e in experiments] == [exp_id_3, store.DEFAULT_EXPERIMENT_ID]
+
+    invoke_gc("--backend-store-uri", uri, "--experiment-ids", exp_id_3, "--older-than", "0s")
+    experiments = store.search_experiments(view_type=ViewType.ALL)
+    assert [e.experiment_id for e in experiments] == [store.DEFAULT_EXPERIMENT_ID]
+
+    exp_id_5 = store.create_experiment("5")
+    store.delete_experiment(exp_id_5)
+    with pytest.raises(MlflowException, match=r"Experiments .+ can be deleted."):
+        invoke_gc(
+            "--backend-store-uri", uri, "--experiment-ids", exp_id_5, "--older-than", "10d10h10m10s"
+        )
+    experiments = store.search_experiments(view_type=ViewType.ALL)
+    assert [e.experiment_id for e in experiments] == [exp_id_5, store.DEFAULT_EXPERIMENT_ID]
+
+
 @pytest.mark.parametrize(
     "enable_mlserver",
     [