mirror of
https://github.com/infiniflow/ragflow.git
synced 2026-06-29 23:41:12 +08:00
## 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 `<lowercase
classname>_<col1>_<col2>` →
**`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) = ("", <provider_uuid>)`
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 <wangq8@outlook.com>
This commit is contained in:
@@ -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 "<table-as-classname>_<col1>_<col2>" 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:
|
||||
|
||||
Reference in New Issue
Block a user