Skip to content

Commit a2acb00

Browse files
committed
fix(config): address review feedback on runtime property reindex (#2098)
- wait_for_completion: gate terminal acceptance on the submitted task_id so a stale ready/failed/cancelled entry left by a prior reindex can no longer be mistaken for this task completing (fixes premature return and spurious raise) - extract shared __submit_property_index_task for update/rebuild; point delete_property_index at the __property_index_path helper - deprecate string index_name: emit Dep030 when a raw string is passed to delete_property_index; the reindex methods take InvertedIndexType - InvertedIndexStatus.type is now the InvertedIndexType enum; parse the status field tolerantly so an unknown state cannot crash the poll loop - validate wait_for_completion; normalize an empty tenants list to no param - tests: async mock coverage, a multi-poll transition regression test, a real in-flight CANCELLED e2e, and split the one-change-per-request PUT test
1 parent de8c7be commit a2acb00

7 files changed

Lines changed: 441 additions & 144 deletions

File tree

integration/test_collection_config.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
)
4848
from weaviate.collections.classes.tenants import Tenant
4949
from weaviate.exceptions import (
50+
ReindexCanceledError,
5051
UnexpectedStatusCodeError,
5152
WeaviateInvalidInputError,
5253
WeaviateUnsupportedFeatureError,
@@ -2863,6 +2864,55 @@ def test_property_reindex_coupled_tokenization_change(
28632864
assert filterable.tokenization == Tokenization.FIELD
28642865

28652866

2867+
def test_property_reindex_cancel_in_flight(collection_factory: CollectionFactory) -> None:
2868+
"""Cancel a live reindex task and observe an actual CANCELLED result + wait-path raise."""
2869+
collection_dummy = collection_factory("dummy")
2870+
if collection_dummy._connection._weaviate_version.is_lower_than(1, 39, 0):
2871+
pytest.skip("Runtime property reindex requires Weaviate >= 1.39.0")
2872+
2873+
collection = collection_factory(
2874+
properties=[
2875+
Property(
2876+
name="name",
2877+
data_type=DataType.TEXT,
2878+
index_filterable=True,
2879+
index_searchable=True,
2880+
tokenization=Tokenization.WORD,
2881+
)
2882+
],
2883+
)
2884+
# a large batch keeps the coupled reindex task live long enough to cancel it
2885+
collection.data.insert_many([{"name": f"object {i}"} for i in range(1000)])
2886+
2887+
task = collection.config.update_property_index(
2888+
"name", InvertedIndexType.SEARCHABLE, tokenization=Tokenization.FIELD
2889+
)
2890+
assert task.status == InvertedIndexTaskStatus.STARTED
2891+
assert task.task_id is not None
2892+
2893+
cancel = collection.config.cancel_property_index_task("name", InvertedIndexType.SEARCHABLE)
2894+
if cancel.status == InvertedIndexTaskStatus.CANCELLED:
2895+
# a live task was stopped: the cancellation targets the same coupled task we submitted
2896+
assert cancel.task_id == task.task_id
2897+
else:
2898+
# the task finished before we could cancel it — nothing left to stop
2899+
assert cancel.status == InvertedIndexTaskStatus.NO_OP
2900+
2901+
# the index settles into a terminal state; if cancelled, waiting on it raises
2902+
if cancel.status == InvertedIndexTaskStatus.CANCELLED:
2903+
try:
2904+
status = collection.config.update_property_index(
2905+
"name",
2906+
InvertedIndexType.SEARCHABLE,
2907+
tokenization=Tokenization.FIELD,
2908+
wait_for_completion=True,
2909+
)
2910+
# a resubmit may relaunch and complete instead of surfacing the cancelled entry
2911+
assert status.status == InvertedIndexState.READY
2912+
except ReindexCanceledError:
2913+
pass
2914+
2915+
28662916
def test_property_reindex_multi_tenant(collection_factory: CollectionFactory) -> None:
28672917
"""Test rangeFilters creation and rebuild with a tenants selection on a multi-tenant collection."""
28682918
collection_dummy = collection_factory("dummy")

mock_tests/test_property_reindex.py

Lines changed: 173 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
import warnings
23
from typing import Generator, Union
34

45
import grpc
@@ -51,16 +52,36 @@ def client_139(
5152
def test_update_property_index_started(
5253
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
5354
) -> None:
55+
"""A single tokenization change is a valid PUT body (the server allows at most one change)."""
5456
weaviate_139_mock.expect_request(
5557
f"{SCHEMA_PATH}/properties/name/index/searchable",
5658
method="PUT",
57-
json={"tokenization": "word", "algorithm": "blockmax"},
59+
json={"tokenization": "word"},
5860
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
5961

6062
task = client_139.collections.use(COLLECTION).config.update_property_index(
6163
"name",
6264
"searchable",
6365
tokenization=Tokenization.WORD,
66+
)
67+
assert task.task_id == TASK_ID
68+
assert task.status == InvertedIndexTaskStatus.STARTED
69+
weaviate_139_mock.check_assertions()
70+
71+
72+
def test_update_property_index_algorithm_only(
73+
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
74+
) -> None:
75+
"""An algorithm-only change is also a valid single-change PUT body."""
76+
weaviate_139_mock.expect_request(
77+
f"{SCHEMA_PATH}/properties/name/index/searchable",
78+
method="PUT",
79+
json={"algorithm": "blockmax"},
80+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
81+
82+
task = client_139.collections.use(COLLECTION).config.update_property_index(
83+
"name",
84+
"searchable",
6485
algorithm="blockmax",
6586
)
6687
assert task.task_id == TASK_ID
@@ -112,14 +133,22 @@ def test_update_property_index_wait_for_completion(
112133
method="PUT",
113134
json={"tokenization": "word"},
114135
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
136+
# the ready entry still carries the submitted task id, so the task_id-gated wait accepts it
115137
weaviate_139_mock.expect_request(f"{SCHEMA_PATH}/indexes", method="GET").respond_with_json(
116138
{
117139
"collection": COLLECTION,
118140
"properties": [
119141
{
120142
"name": "name",
121143
"dataType": "text",
122-
"indexes": [{"type": "searchable", "status": "ready", "tokenization": "word"}],
144+
"indexes": [
145+
{
146+
"type": "searchable",
147+
"status": "ready",
148+
"taskId": TASK_ID,
149+
"tokenization": "word",
150+
}
151+
],
123152
}
124153
],
125154
}
@@ -378,6 +407,8 @@ def test_get_property_indexes(
378407
assert name.description == "a text property"
379408
assert len(name.indexes) == 2
380409
searchable, filterable = name.indexes
410+
# `type` parses strictly into the InvertedIndexType enum; str-enum equality still holds
411+
assert searchable.type is InvertedIndexType.SEARCHABLE
381412
assert searchable.type == "searchable"
382413
assert searchable.status == InvertedIndexState.INDEXING
383414
assert searchable.progress == 0.5
@@ -591,6 +622,26 @@ def test_delete_property_index_surfaces_server_message(
591622
weaviate_139_mock.check_assertions()
592623

593624

625+
def test_delete_property_index_string_deprecation_warning(
626+
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
627+
) -> None:
628+
"""A raw-string index_name on delete_property_index warns; the enum form does not."""
629+
weaviate_139_mock.expect_request(
630+
f"{SCHEMA_PATH}/properties/name/index/searchable",
631+
method="DELETE",
632+
).respond_with_json({}, status=200)
633+
634+
config = client_139.collections.use(COLLECTION).config
635+
with pytest.warns(DeprecationWarning, match="Dep030"):
636+
assert config.delete_property_index("name", "searchable") is True
637+
638+
# the InvertedIndexType form is the supported path and must not warn
639+
with warnings.catch_warnings():
640+
warnings.simplefilter("error")
641+
assert config.delete_property_index("name", InvertedIndexType.SEARCHABLE) is True
642+
weaviate_139_mock.check_assertions()
643+
644+
594645
def test_property_reindex_invalid_input(
595646
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
596647
) -> None:
@@ -606,6 +657,126 @@ def test_property_reindex_invalid_input(
606657
weaviate_139_mock.check_assertions()
607658

608659

660+
def _single_index_payload(entry: dict) -> dict:
661+
return {
662+
"collection": COLLECTION,
663+
"properties": [{"name": "name", "dataType": "text", "indexes": [entry]}],
664+
}
665+
666+
667+
def test_update_property_index_wait_polls_until_submitted_task_ready(
668+
weaviate_139_mock: HTTPServer, client_139: weaviate.WeaviateClient
669+
) -> None:
670+
"""Regression for the task_id gate (fix #1).
671+
672+
The wait must ignore a stale ``ready`` left by a PRIOR reindex and only accept the ``ready``
673+
that follows the submitted task's own progress — never returning early. If the task_id gate
674+
were removed, the first (stale) ready would be returned and the tokenization assertion below
675+
would fail.
676+
"""
677+
weaviate_139_mock.expect_ordered_request(
678+
f"{SCHEMA_PATH}/properties/name/index/searchable",
679+
method="PUT",
680+
json={"tokenization": "field"},
681+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
682+
# poll 1: a stale ready from a prior reindex (different task id) — must NOT be accepted
683+
weaviate_139_mock.expect_ordered_request(
684+
f"{SCHEMA_PATH}/indexes", method="GET"
685+
).respond_with_json(
686+
_single_index_payload(
687+
{
688+
"type": "searchable",
689+
"status": "ready",
690+
"taskId": "stale-task",
691+
"tokenization": "word",
692+
}
693+
)
694+
)
695+
# poll 2: our task mid-finalize (indexing @ 1.0 carrying our task id)
696+
weaviate_139_mock.expect_ordered_request(
697+
f"{SCHEMA_PATH}/indexes", method="GET"
698+
).respond_with_json(
699+
_single_index_payload(
700+
{
701+
"type": "searchable",
702+
"status": "indexing",
703+
"progress": 1.0,
704+
"taskId": TASK_ID,
705+
"tokenization": "word",
706+
"targetTokenization": "field",
707+
}
708+
)
709+
)
710+
# poll 3: flipped to plain ready with the new tokenization
711+
weaviate_139_mock.expect_ordered_request(
712+
f"{SCHEMA_PATH}/indexes", method="GET"
713+
).respond_with_json(
714+
_single_index_payload({"type": "searchable", "status": "ready", "tokenization": "field"})
715+
)
716+
717+
status = client_139.collections.use(COLLECTION).config.update_property_index(
718+
"name",
719+
InvertedIndexType.SEARCHABLE,
720+
tokenization=Tokenization.FIELD,
721+
wait_for_completion=True,
722+
)
723+
assert status.status == InvertedIndexState.READY
724+
assert status.tokenization == Tokenization.FIELD
725+
weaviate_139_mock.check_assertions()
726+
727+
728+
@pytest.mark.asyncio
729+
async def test_update_property_index_async(
730+
weaviate_139_mock: HTTPServer, start_grpc_server: grpc.Server
731+
) -> None:
732+
"""The async fork of update_property_index submits and waits via the task_id gate."""
733+
weaviate_139_mock.expect_request(
734+
f"{SCHEMA_PATH}/properties/name/index/searchable",
735+
method="PUT",
736+
json={"tokenization": "word"},
737+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
738+
weaviate_139_mock.expect_request(f"{SCHEMA_PATH}/indexes", method="GET").respond_with_json(
739+
_single_index_payload(
740+
{"type": "searchable", "status": "ready", "taskId": TASK_ID, "tokenization": "word"}
741+
)
742+
)
743+
744+
async with weaviate.use_async_with_local(
745+
host=MOCK_IP, port=MOCK_PORT, grpc_port=MOCK_PORT_GRPC
746+
) as client:
747+
status = await client.collections.use(COLLECTION).config.update_property_index(
748+
"name",
749+
InvertedIndexType.SEARCHABLE,
750+
tokenization=Tokenization.WORD,
751+
wait_for_completion=True,
752+
)
753+
assert status.status == InvertedIndexState.READY
754+
assert status.tokenization == Tokenization.WORD
755+
weaviate_139_mock.check_assertions()
756+
757+
758+
@pytest.mark.asyncio
759+
async def test_rebuild_property_index_async(
760+
weaviate_139_mock: HTTPServer, start_grpc_server: grpc.Server
761+
) -> None:
762+
"""The async fork of rebuild_property_index submits a POST and returns the task."""
763+
weaviate_139_mock.expect_request(
764+
f"{SCHEMA_PATH}/properties/name/index/searchable/rebuild",
765+
method="POST",
766+
json={},
767+
).respond_with_json({"taskId": TASK_ID, "status": "STARTED"}, status=202)
768+
769+
async with weaviate.use_async_with_local(
770+
host=MOCK_IP, port=MOCK_PORT, grpc_port=MOCK_PORT_GRPC
771+
) as client:
772+
task = await client.collections.use(COLLECTION).config.rebuild_property_index(
773+
"name", InvertedIndexType.SEARCHABLE
774+
)
775+
assert task.task_id == TASK_ID
776+
assert task.status == InvertedIndexTaskStatus.STARTED
777+
weaviate_139_mock.check_assertions()
778+
779+
609780
def test_property_reindex_unsupported_version(
610781
weaviate_client: weaviate.WeaviateClient,
611782
) -> None:

weaviate/collections/classes/config.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,12 @@
110110
"high",
111111
]
112112

113+
# Deprecated: this string alias is superseded by the ``InvertedIndexType`` enum and will be
114+
# removed in a future release. Prefer ``InvertedIndexType`` for property index type arguments;
115+
# the string form is still accepted by ``delete_property_index`` (which emits a
116+
# ``DeprecationWarning``) for backwards compatibility. ``typing_extensions.deprecated`` (PEP 702)
117+
# cannot annotate a bare type alias — it decorates classes/functions/overloads — so the
118+
# deprecation is surfaced via this comment and the runtime warning on the accepting method.
113119
IndexName: TypeAlias = Literal[
114120
"searchable",
115121
"filterable",
@@ -2322,8 +2328,14 @@ class _InvertedIndexTask(_ConfigBase):
23222328

23232329
@dataclass
23242330
class _InvertedIndexStatus(_ConfigBase):
2325-
type: IndexName # noqa: A003
2326-
status: InvertedIndexState
2331+
"""The status of a single property index as reported by the index status endpoint.
2332+
2333+
Known `status` values parse to `InvertedIndexState`; unknown server values pass through
2334+
as plain strings, so that polling a status endpoint never crashes on a new state name.
2335+
"""
2336+
2337+
type: InvertedIndexType # noqa: A003
2338+
status: Union[InvertedIndexState, str]
23272339
progress: Optional[float]
23282340
task_id: Optional[str]
23292341
tokenization: Optional[Tokenization]

weaviate/collections/classes/config_methods.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
from weaviate.collections.classes.config import (
55
DataType,
66
GenerativeSearches,
7-
IndexName,
87
InvertedIndexState,
98
InvertedIndexTaskStatus,
9+
InvertedIndexType,
1010
PQEncoderDistribution,
1111
PQEncoderType,
1212
ReplicationDeletionStrategy,
@@ -583,9 +583,18 @@ def _inverted_index_task_from_json(response: Dict[str, Any]) -> _InvertedIndexTa
583583
def _inverted_index_status_from_json(index: Dict[str, Any]) -> _InvertedIndexStatus:
584584
tokenization = index.get("tokenization")
585585
target_tokenization = index.get("targetTokenization")
586+
raw_status = index["status"]
587+
try:
588+
status: Union[InvertedIndexState, str] = InvertedIndexState(raw_status)
589+
except ValueError:
590+
# the spec declares the field open-vocabulary; pass unknown values through so that
591+
# polling a status endpoint never crashes on a newly-added state name
592+
status = raw_status
586593
return _InvertedIndexStatus(
587-
type=cast(IndexName, index["type"]),
588-
status=InvertedIndexState(index["status"]),
594+
# `type` is closed-vocabulary and server-canonical (always filterable|searchable|
595+
# rangeFilters), so parse it strictly into the enum, matching the file convention.
596+
type=InvertedIndexType(index["type"]),
597+
status=status,
589598
progress=index.get("progress"),
590599
task_id=index.get("taskId"),
591600
tokenization=Tokenization(tokenization) if tokenization is not None else None,

0 commit comments

Comments
 (0)