-
Notifications
You must be signed in to change notification settings - Fork 7.6k
feat: optimize metadata counting using Qdrant server-side facets #1868
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7a2aa08
81962b7
759f0cd
ee594e8
6e51baf
0a39f2a
26896a7
edb76db
0a6858f
0a12b91
c732fee
edf8087
7ca1f05
e9135e5
0a94456
a0b0df4
4390e41
992a078
8dcdc76
4e3d90e
5eb8427
ea66851
2505325
de692be
39c8929
25c86fb
b48bd73
8c0d53c
b939089
7180f1b
65696d5
8f549d4
4de1d2d
350023f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -40,6 +40,7 @@ | |
| PalaceNotFoundError, | ||
| PalaceRef, | ||
| QueryResult, | ||
| UnsupportedCapabilityError, | ||
| UnsupportedFilterError, | ||
| _IncludeSpec, | ||
| ) | ||
|
|
@@ -543,6 +544,38 @@ def count_points(self, collection: str) -> int: | |
| result = response.get("result") or {} | ||
| return int(result.get("count") or 0) | ||
|
|
||
| def facet_counts( | ||
| self, | ||
| collection: str, | ||
| *, | ||
| field: str, | ||
| qdrant_filter: Optional[dict] = None, | ||
| limit: int = 1000, | ||
| ) -> dict[str, int]: | ||
| body: dict[str, Any] = { | ||
| "key": field, | ||
| "exact": True, | ||
| "limit": limit, | ||
| } | ||
|
|
||
| if qdrant_filter: | ||
| body["filter"] = qdrant_filter | ||
|
|
||
| response = self.request( | ||
| "POST", | ||
| f"/collections/{urlparse.quote(collection, safe='')}/facet", | ||
| body=body, | ||
| ) | ||
|
|
||
| result = response.get("result") or {} | ||
| hits = result.get("hits") or [] | ||
|
|
||
| return { | ||
| str(hit["value"]): int(hit.get("count") or 0) | ||
| for hit in hits | ||
| if hit.get("value") is not None | ||
| } | ||
|
Prayaksh marked this conversation as resolved.
Prayaksh marked this conversation as resolved.
Comment on lines
+573
to
+577
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is a syntax error in the return {
str(hit["value"]): int(hit.get("count") or 0) for hit in hits if hit.get("value") is not None
} |
||
|
|
||
| def delete_collection(self, collection: str) -> None: | ||
| self.request("DELETE", f"/collections/{urlparse.quote(collection, safe='')}") | ||
|
|
||
|
|
@@ -1035,6 +1068,34 @@ def get_all_metadata(self, where: Optional[dict] = None) -> list[dict]: | |
| rows = self._rows(where=where) | ||
| return [row["metadata"] for row in rows] | ||
|
|
||
| def facet_counts( | ||
| self, | ||
| field: str, | ||
| where: Optional[dict] = None, | ||
| limit: int = 1000, | ||
| ) -> dict[str, int]: | ||
| self._ensure_open() | ||
| # Validate the filter before the existence short-circuit so an | ||
| # unsupported local-only filter raises regardless of whether the | ||
| # collection has been materialized yet — matching the order used by | ||
| # get()/lexical_search() above (#1835 review). | ||
| _validate_where(where) | ||
| if _requires_local_filter(where): | ||
| raise UnsupportedCapabilityError("facet_counts does not support local-only filters") | ||
| if not self._remote_exists(): | ||
| if self._marker_exists(): | ||
| raise CollectionNotInitializedError(self._collection_name) | ||
| return {} | ||
|
|
||
|
Comment on lines
+1077
to
+1089
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The facet_counts method does not check if the remote collection exists before making the REST request. If the collection is not initialized, this will result in an unhandled _QdrantHTTPError (404) instead of raising CollectionNotInitializedError (which is consistent with other methods like count, get, and delete). Adding a self._remote_exists() check aligns this method with the rest of the collection interface. self._ensure_open()
if not self._remote_exists():
if self._marker_exists():
raise CollectionNotInitializedError(self._collection_name)
return {}
_validate_where(where)
if _requires_local_filter(where, None):
raise UnsupportedCapabilityError('facet_counts does not support local-only filters') |
||
| q_filter = _qdrant_filter(where) | ||
|
|
||
| return self._client.facet_counts( | ||
| self._remote_collection, | ||
| field=f"{_PAYLOAD_METADATA}.{field}", | ||
| qdrant_filter=q_filter, | ||
| limit=limit, | ||
| ) | ||
|
Prayaksh marked this conversation as resolved.
|
||
|
|
||
| def delete(self, *, ids=None, where=None): | ||
| _validate_where(where) | ||
| if not self._remote_exists(): | ||
|
|
@@ -1125,6 +1186,7 @@ class QdrantBackend(BaseBackend): | |
| "supports_embeddings_out", | ||
| "supports_metadata_filters", | ||
| "supports_lexical_search", | ||
| "supports_metadata_facets", | ||
| "supports_namespace_isolation", | ||
| "server_mode", | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1205,6 +1205,15 @@ def _fetch_all_metadata(col, where=None): | |
| return all_meta | ||
|
|
||
|
|
||
| def _supports_metadata_facets(col) -> bool: | ||
| """Return True if the collection's backend implements metadata facets.""" | ||
| backend = getattr(col, "_backend", None) | ||
| if backend is None: | ||
| return False | ||
| capabilities = getattr(backend, "capabilities", None) | ||
| return isinstance(capabilities, (set, frozenset)) and "supports_metadata_facets" in capabilities | ||
|
|
||
|
|
||
| _metadata_cache = None | ||
| _metadata_cache_time = 0 | ||
| _METADATA_CACHE_TTL = 5.0 # seconds | ||
|
|
@@ -1641,13 +1650,47 @@ def tool_status(): | |
| "backend": _selected_backend_name(), | ||
| } | ||
| try: | ||
| all_meta = _get_cached_metadata(col) | ||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| r = m.get("room", "unknown") | ||
| wings[w] = wings.get(w, 0) + 1 | ||
| rooms[r] = rooms.get(r, 0) + 1 | ||
| if _supports_metadata_facets(col): | ||
| try: | ||
| temp_wings = col.facet_counts("wing") | ||
| wings.update(temp_wings) | ||
| try: | ||
| unknown_wings = count - sum(temp_wings.values()) | ||
| if unknown_wings > 0: | ||
| wings["unknown"] = wings.get("unknown", 0) + unknown_wings | ||
| except (TypeError, ValueError): | ||
| pass | ||
|
|
||
| temp_rooms = col.facet_counts("room") | ||
| rooms.update(temp_rooms) | ||
| try: | ||
| unknown_rooms = count - sum(temp_rooms.values()) | ||
| if unknown_rooms > 0: | ||
| rooms["unknown"] = rooms.get("unknown", 0) + unknown_rooms | ||
| except (TypeError, ValueError): | ||
| pass | ||
|
|
||
|
Comment on lines
+1654
to
+1672
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If col.facet_counts('wing') succeeds but a subsequent call (like col.facet_counts('room') or col.count()) raises an exception, the wings dictionary will be partially populated. When the exception is caught, the fallback client-side loop will add counts on top of the already-populated wings dictionary, leading to double-counting and corrupted state. Using temporary dictionaries and only updating the main wings and rooms dictionaries upon complete success prevents this issue. try:
temp_wings = col.facet_counts('wing')
try:
unknown_wings = col.count() - sum(temp_wings.values())
if unknown_wings > 0:
temp_wings['unknown'] = unknown_wings
except (TypeError, ValueError):
pass
temp_rooms = col.facet_counts('room')
try:
unknown_rooms = col.count() - sum(temp_rooms.values())
if unknown_rooms > 0:
temp_rooms['unknown'] = unknown_rooms
except (TypeError, ValueError):
pass
wings.update(temp_wings)
rooms.update(temp_rooms) |
||
| except Exception as e: | ||
| logger.warning( | ||
| "Failed to fetch metadata facets, falling back to client-side loop: %s", e | ||
| ) | ||
| rooms.clear() | ||
| wings.clear() | ||
| all_meta = _get_cached_metadata(col) | ||
|
Prayaksh marked this conversation as resolved.
|
||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| r = m.get("room", "unknown") | ||
| wings[w] = wings.get(w, 0) + 1 | ||
| rooms[r] = rooms.get(r, 0) + 1 | ||
| else: | ||
| all_meta = _get_cached_metadata(col) | ||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| r = m.get("room", "unknown") | ||
| wings[w] = wings.get(w, 0) + 1 | ||
|
Prayaksh marked this conversation as resolved.
Prayaksh marked this conversation as resolved.
|
||
| rooms[r] = rooms.get(r, 0) + 1 | ||
|
Prayaksh marked this conversation as resolved.
Prayaksh marked this conversation as resolved.
|
||
| except Exception as e: | ||
| logger.exception("tool_status metadata fetch failed") | ||
| result["error"] = str(e) | ||
|
|
@@ -1702,11 +1745,28 @@ def tool_list_wings(): | |
| wings = {} | ||
| result = {"wings": wings} | ||
| try: | ||
| all_meta = _get_cached_metadata(col) | ||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| wings[w] = wings.get(w, 0) + 1 | ||
| try: | ||
| if not _supports_metadata_facets(col): | ||
| raise ValueError("facets not supported") | ||
| temp_wings = col.facet_counts("wing") | ||
| wings.update(temp_wings) | ||
| try: | ||
| unknown_wings = col.count() - sum(temp_wings.values()) | ||
| if unknown_wings > 0: | ||
| wings["unknown"] = wings.get("unknown", 0) + unknown_wings | ||
| except (TypeError, ValueError): | ||
| pass | ||
| except Exception as e: | ||
| if _supports_metadata_facets(col): | ||
| logger.warning( | ||
| "Failed to fetch metadata facets, falling back to client-side loop: %s", e | ||
| ) | ||
| wings.clear() | ||
| all_meta = _get_cached_metadata(col) | ||
|
Prayaksh marked this conversation as resolved.
|
||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| wings[w] = wings.get(w, 0) + 1 | ||
|
Prayaksh marked this conversation as resolved.
Prayaksh marked this conversation as resolved.
|
||
| except Exception as e: | ||
| logger.exception("tool_list_wings metadata fetch failed") | ||
| result["error"] = str(e) | ||
|
|
@@ -1734,13 +1794,34 @@ def tool_list_rooms(wing: str = None): | |
| return _collection_error_or_no_palace() | ||
| rooms = {} | ||
| result = {"wing": wing or "all", "rooms": rooms} | ||
| where = {"wing": wing} if wing else None | ||
| try: | ||
| where = {"wing": wing} if wing else None | ||
| all_meta = _fetch_all_metadata(col, where=where) | ||
| for m in all_meta: | ||
| m = m or {} | ||
| r = m.get("room", "unknown") | ||
| rooms[r] = rooms.get(r, 0) + 1 | ||
| try: | ||
| if not _supports_metadata_facets(col): | ||
| raise ValueError("facets not supported") | ||
| temp_rooms = col.facet_counts("room", where=where) | ||
| rooms.update(temp_rooms) | ||
| try: | ||
| if wing: | ||
| wing_count = col.facet_counts("wing", where={"wing": wing}).get(wing, 0) | ||
| unknown_rooms = wing_count - sum(temp_rooms.values()) | ||
| else: | ||
| unknown_rooms = col.count() - sum(temp_rooms.values()) | ||
| if unknown_rooms > 0: | ||
| rooms["unknown"] = rooms.get("unknown", 0) + unknown_rooms | ||
| except (TypeError, ValueError): | ||
| pass | ||
| except Exception as e: | ||
| if _supports_metadata_facets(col): | ||
| logger.warning( | ||
| "Failed to fetch metadata facets, falling back to client-side loop: %s", e | ||
| ) | ||
| rooms.clear() | ||
| all_meta = _fetch_all_metadata(col, where=where) | ||
|
Prayaksh marked this conversation as resolved.
|
||
| for m in all_meta: | ||
| m = m or {} | ||
| r = m.get("room", "unknown") | ||
| rooms[r] = rooms.get(r, 0) + 1 | ||
|
Prayaksh marked this conversation as resolved.
Prayaksh marked this conversation as resolved.
|
||
| except Exception as e: | ||
| logger.exception("tool_list_rooms metadata fetch failed") | ||
| result["error"] = str(e) | ||
|
|
@@ -1759,14 +1840,42 @@ def tool_get_taxonomy(): | |
| taxonomy = {} | ||
| result = {"taxonomy": taxonomy} | ||
| try: | ||
| all_meta = _get_cached_metadata(col) | ||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| r = m.get("room", "unknown") | ||
| if w not in taxonomy: | ||
| taxonomy[w] = {} | ||
| taxonomy[w][r] = taxonomy[w].get(r, 0) + 1 | ||
| try: | ||
| if not _supports_metadata_facets(col): | ||
| raise ValueError("facets not supported") | ||
| from concurrent.futures import ThreadPoolExecutor | ||
|
|
||
| wing_counts = col.facet_counts("wing") | ||
| wings = list(wing_counts.keys()) | ||
| temp_taxonomy = {} | ||
| with ThreadPoolExecutor(max_workers=max(1, min(8, len(wings)))) as executor: | ||
| futures = { | ||
| wing: executor.submit(col.facet_counts, "room", where={"wing": wing}) | ||
| for wing in wings | ||
| } | ||
| for wing, future in futures.items(): | ||
| room_counts = future.result() | ||
| try: | ||
| unknown_rooms = wing_counts[wing] - sum(room_counts.values()) | ||
| if unknown_rooms > 0: | ||
| room_counts["unknown"] = room_counts.get("unknown", 0) + unknown_rooms | ||
| except (TypeError, ValueError): | ||
| pass | ||
| temp_taxonomy[wing] = room_counts | ||
|
Prayaksh marked this conversation as resolved.
|
||
| taxonomy.update(temp_taxonomy) | ||
| except Exception as e: | ||
| if _supports_metadata_facets(col): | ||
| logger.warning( | ||
| "Failed to fetch metadata facets, falling back to client-side loop: %s", e | ||
| ) | ||
| all_meta = _get_cached_metadata(col) | ||
| for m in all_meta: | ||
| m = m or {} | ||
| w = m.get("wing", "unknown") | ||
| r = m.get("room", "unknown") | ||
| if w not in taxonomy: | ||
| taxonomy[w] = {} | ||
| taxonomy[w][r] = taxonomy[w].get(r, 0) + 1 | ||
|
Prayaksh marked this conversation as resolved.
Prayaksh marked this conversation as resolved.
|
||
| except Exception as e: | ||
| logger.exception("tool_get_taxonomy metadata fetch failed") | ||
| result["error"] = str(e) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.