From b978e262085716dd0b5db9280c604e0a567e12b8 Mon Sep 17 00:00:00 2001 From: Rene Arredondo <120709323+Rene0422@users.noreply.github.com> Date: Thu, 11 Jun 2026 00:47:12 -0700 Subject: [PATCH] fix(db): drop Peewee-auto-named unique index on tenant_model_instance (#15699) (#15879) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Fixes #15699. User upgrades to v0.25.6 against an existing MySQL database, tries to add an Ollama provider instance, and gets: ``` MySQL IntegrityError: Duplicate entry 'dbaafbfe608a11f1a5516d6066988224' for key 'tenant_model_instance.tenantmodelinstance_api_key_provider_id' ``` The route at [api/apps/restful_apis/provider_api.py:354](api/apps/restful_apis/provider_api.py#L354) catches it and returns `get_error_data_result(message="Internal server error")` — which by RAGFlow's convention is HTTP 200 with an error `code` on the body — hence the reporter's "200 status code but the database errored" complaint. ### Root cause The provider-instance refactor in [PR #15460](https://github.com/infiniflow/ragflow/pull/15460) dropped the unique-compound-index tuple from `TenantModelInstance`: ```python # Removed in #15460 class Meta: db_table = "tenant_model_instance" indexes = ( (("api_key", "provider_id"), True), # unique ) ``` and added a one-shot drop in `migrate_db()` for existing databases. But the drop targets the wrong index name: ```python # Before this PR — wrong name for table_name, index_name in [ ("tenant_model_instance", "idx_api_key_provider_id"), # ← doesn't exist ("tenant_model", "idx_provider_model_instance"), ]: ``` Peewee's auto-derived index name is `__` → **`tenantmodelinstance_api_key_provider_id`**, which matches the user's error verbatim. The drop raises `OperationalError: 1091 (HY000): Can't DROP …`, the surrounding `except` clause at [db_models.py:1736](api/db/db_models.py#L1736) swallows it as expected-on-fresh-installs, and the legacy unique index lives on indefinitely. ### Why Ollama hits it specifically Ollama doesn't require an API key. The form posts `api_key: ""`. The app-layer dedupe at [provider_api_service.py:288-292](api/apps/services/provider_api_service.py#L288-L292): ```python api_key_str = "" if api_key: # ← skipped for "" ... same_key_instance = TenantModelInstanceService.get_by_provider_id_and_api_key(...) if same_key_instance: return False, f"Already exist instance: ... with api_key {api_key}" ``` falls through for empty keys. Control reaches `TenantModelInstanceService.create_instance(..., api_key="")` which inserts a row whose `(api_key, provider_id) = ("", )` collides with any prior Ollama row that already shipped that same pair → the still-present unique index throws. (`dbaafbfe608a11f1a5516d6066988224` in the user's error is the duplicated `provider_id` UUID, paired with the empty `api_key`.) ### Fix Add the Peewee auto-name alongside the existing `idx_*` entry so the migration finally drops the obsolete index on next restart: ```python legacy_indexes = [ ("tenant_model_instance", "idx_api_key_provider_id"), ("tenant_model_instance", "tenantmodelinstance_api_key_provider_id"), # ← added ("tenant_model", "idx_provider_model_instance"), ] ``` The surrounding `try/except (OperationalError, ProgrammingError)` matches `1091` / `can't DROP` / `does not exist` and treats them as success, so every state is idempotent (see Test plan). ### Idempotency matrix | Database state | First entry (`idx_api_key_provider_id`) | New entry (`tenantmodelinstance_api_key_provider_id`) | | --- | --- | --- | | Fresh install (≥ #15460) — neither index exists | `1091` → swallowed | `1091` → swallowed | | Upgraded from before dc4b82523 (the user's case) — auto-name present | `1091` → swallowed | **drops the index** | | Upgraded after a manual rename to `idx_*` | drops the index | `1091` → swallowed | | Re-run of `migrate_db()` after either of the above | `1091` → swallowed | `1091` → swallowed | No rollback hazard: nothing depends on this unique constraint anymore (`create_instance` dedupes by `instance_name` via `duplicate_name`, see [tenant_model_instance_service.py:27](api/db/services/tenant_model_instance_service.py#L27)). ### What this PR does NOT change - **`provider_api_service.create_provider_instance`** — its `if api_key:` gate is correct *for the post-migration world*: multiple Ollama instances with empty keys under one provider are legitimate, so we shouldn't tighten the app-layer check. - **`TenantModelInstance` Peewee model** — the `indexes` tuple was already removed in #15460. New databases never get the constraint in the first place. - **The `except → get_error_data_result` → HTTP 200 pattern at `provider_api.py:354`** — that's a project-wide convention; changing one route to HTTP 500 would be inconsistent and out of scope. ## Test plan - [ ] **Reproducer (pre-fix):** on a database originally created before #15460, configure an Ollama provider with an empty `api_key`, then try to create a *second* instance under the same provider — confirm the `Duplicate entry … 'tenantmodelinstance_api_key_provider_id'` error in the server log. - [ ] **Verify the index is present pre-restart:** `SHOW INDEX FROM tenant_model_instance WHERE Key_name = 'tenantmodelinstance_api_key_provider_id';` — non-empty result. - [ ] **Restart with the fix applied:** server starts cleanly, `migrate_db()` runs, no `Failed to drop index` in critical logs. - [ ] **Verify the index is gone post-restart:** same `SHOW INDEX` query — empty result. - [ ] **Re-run the reproducer:** two Ollama instances under the same provider, both `api_key=""`, both succeed. - [ ] **Restart a second time** — no new errors; the matching `1091` swallow keeps the migration idempotent. - [ ] **Fresh install smoke test:** drop the DB volume, start clean — no `1091` noise (the new index never existed), no functional regression. ## Files changed - [api/db/db_models.py](api/db/db_models.py) — extend the legacy-index drop list with `tenantmodelinstance_api_key_provider_id`; refactor the inline list to a named `legacy_indexes` local with a comment pointing at #15460 and #15699. ### Type of change - [x] Bug Fix (non-breaking change which fixes an issue) - [ ] New Feature (non-breaking change which adds functionality) - [ ] Documentation Update - [ ] Refactoring - [ ] Performance Improvement - [ ] Other (please describe): Co-authored-by: Wang Qi --- api/db/db_models.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/api/db/db_models.py b/api/db/db_models.py index a81b32c22b..ee96c705fc 100644 --- a/api/db/db_models.py +++ b/api/db/db_models.py @@ -1728,7 +1728,20 @@ def migrate_db(): alter_db_column_type(migrator, "document", "size", BigIntegerField(default=0, index=True)) alter_db_column_type(migrator, "file", "size", BigIntegerField(default=0, index=True)) alter_db_add_column(migrator, "tenant", "ocr_id", CharField(max_length=128, null=True, help_text="default ocr model ID", index=True)) - for table_name, index_name in [("tenant_model_instance", "idx_api_key_provider_id"), ("tenant_model", "idx_provider_model_instance")]: + # Drop both the explicit "idx_*" name from later migrations AND the + # Peewee-auto-derived "__" name from the + # original TenantModelInstance definition (commit dc4b82523). Databases + # created before #15460 dropped the model's `indexes = ((...,), True)` + # tuple still carry the auto-named compound unique index, which makes a + # second instance with an empty api_key (e.g. Ollama) fail with + # "Duplicate entry ... for key 'tenantmodelinstance_api_key_provider_id'" + # — see #15699. + legacy_indexes = [ + ("tenant_model_instance", "idx_api_key_provider_id"), + ("tenant_model_instance", "tenantmodelinstance_api_key_provider_id"), + ("tenant_model", "idx_provider_model_instance"), + ] + for table_name, index_name in legacy_indexes: try: migrate(migrator.drop_index(table_name, index_name)) except (OperationalError, ProgrammingError) as ex: