Skip to content

Commit b1b28f9

Browse files
committed
fix: single-flight discovery engine mode detection
1 parent d4ba521 commit b1b28f9

2 files changed

Lines changed: 88 additions & 13 deletions

File tree

src/google/adk/tools/discovery_engine_search_tool.py

Lines changed: 25 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import json
2020
import logging
2121
import re
22+
import threading
2223
from typing import Any
2324
from typing import Optional
2425

@@ -172,6 +173,7 @@ def __init__(
172173
self._filter = filter
173174
self._max_results = max_results
174175
self._search_result_mode = search_result_mode
176+
self._search_result_mode_lock = threading.Lock()
175177
self._location = location
176178

177179
credentials, _ = google.auth.default()
@@ -204,19 +206,29 @@ def discovery_engine_search(
204206
if mode is not None:
205207
return self._do_search(query, mode)
206208

207-
# Auto-detect: try CHUNKS first, fall back to DOCUMENTS
208-
# if the datastore requires it.
209-
try:
210-
return self._do_search(query, SearchResultMode.CHUNKS)
211-
except GoogleAPICallError as e:
212-
if _STRUCTURED_STORE_ERROR_PATTERN.search(str(e)):
213-
logger.info(
214-
'CHUNKS mode failed for structured datastore,'
215-
' retrying with DOCUMENTS mode.'
216-
)
217-
self._search_result_mode = SearchResultMode.DOCUMENTS
218-
return self._do_search(query, SearchResultMode.DOCUMENTS)
219-
raise
209+
# Auto-detect is per datastore, not per query. Keep the probe
210+
# single-flight so concurrent first calls do not all spend a CHUNKS
211+
# request before learning the same DOCUMENTS fallback.
212+
with self._search_result_mode_lock:
213+
mode = self._search_result_mode
214+
if mode is None:
215+
try:
216+
result = self._do_search(query, SearchResultMode.CHUNKS)
217+
except GoogleAPICallError as e:
218+
if _STRUCTURED_STORE_ERROR_PATTERN.search(str(e)):
219+
logger.info(
220+
'CHUNKS mode failed for structured datastore,'
221+
' retrying with DOCUMENTS mode.'
222+
)
223+
self._search_result_mode = SearchResultMode.DOCUMENTS
224+
mode = SearchResultMode.DOCUMENTS
225+
else:
226+
raise
227+
else:
228+
self._search_result_mode = SearchResultMode.CHUNKS
229+
return result
230+
231+
return self._do_search(query, mode)
220232
except GoogleAPICallError as e:
221233
return {'status': 'error', 'error_message': str(e)}
222234

tests/unittests/tools/test_discovery_engine_search_tool.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15+
import concurrent.futures
16+
import threading
17+
import time
1518
from unittest import mock
1619

1720
from google.adk.tools import discovery_engine_search_tool
@@ -489,6 +492,66 @@ def test_auto_detect_falls_back_to_documents(self, mock_search_client):
489492
# Mode should be persisted so subsequent calls skip the retry.
490493
assert tool._search_result_mode == SearchResultMode.DOCUMENTS
491494

495+
@mock.patch.object(
496+
discoveryengine,
497+
"SearchServiceClient",
498+
)
499+
def test_auto_detect_singleflights_structured_fallback(
500+
self, mock_search_client
501+
):
502+
"""Concurrent cold calls should share one CHUNKS probe."""
503+
spec_cls = discoveryengine.SearchRequest.ContentSearchSpec
504+
worker_count = 8
505+
start_barrier = threading.Barrier(worker_count)
506+
search_lock = threading.Lock()
507+
search_modes = []
508+
structured_error = exceptions.InvalidArgument(
509+
"`content_search_spec.search_result_mode` must be set to"
510+
" SearchRequest.ContentSearchSpec.SearchResultMode.DOCUMENTS"
511+
" when the engine contains structured data store."
512+
)
513+
mock_doc = discoveryengine.Document(
514+
name="projects/p/locations/l/doc1",
515+
id="doc1",
516+
struct_data={
517+
"title": "Jira Issue",
518+
"uri": "https://jira.example.com/123",
519+
"summary": "Bug fix",
520+
},
521+
)
522+
mock_doc_response = discoveryengine.SearchResponse()
523+
mock_doc_response.results = [
524+
discoveryengine.SearchResponse.SearchResult(document=mock_doc)
525+
]
526+
527+
def search(request):
528+
mode = request.content_search_spec.search_result_mode
529+
with search_lock:
530+
search_modes.append(mode)
531+
if mode == spec_cls.SearchResultMode.CHUNKS:
532+
time.sleep(0.05)
533+
raise structured_error
534+
return mock_doc_response
535+
536+
mock_search_client.return_value.search.side_effect = search
537+
tool = DiscoveryEngineSearchTool(data_store_id="test_data_store")
538+
539+
def run_search(index):
540+
start_barrier.wait(timeout=5)
541+
return tool.discovery_engine_search(f"test query {index}")
542+
543+
with concurrent.futures.ThreadPoolExecutor(
544+
max_workers=worker_count
545+
) as executor:
546+
results = list(executor.map(run_search, range(worker_count)))
547+
548+
assert all(result["status"] == "success" for result in results)
549+
assert search_modes.count(spec_cls.SearchResultMode.CHUNKS) == 1
550+
assert (
551+
search_modes.count(spec_cls.SearchResultMode.DOCUMENTS) == worker_count
552+
)
553+
assert tool._search_result_mode == SearchResultMode.DOCUMENTS
554+
492555
@mock.patch.object(
493556
discoveryengine,
494557
"SearchServiceClient",

0 commit comments

Comments
 (0)