Skip to content

Commit 3a9a88c

Browse files
he-yufengcopybara-github
authored andcommitted
fix: single-flight Discovery Engine mode detection
Merge #6106 Make the auto-detect probe single-flight per tool instance. The first cold caller detects the datastore mode and caches it; concurrent callers re-check the cached mode after the lock and go straight to the detected mode. The successful CHUNKS case is cached too, since the result mode is a datastore property rather than a query property. Closes: #6101 PiperOrigin-RevId: 954907232
1 parent b315b00 commit 3a9a88c

2 files changed

Lines changed: 122 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

@@ -184,6 +185,7 @@ def __init__(
184185
self._filter = filter
185186
self._max_results = max_results
186187
self._search_result_mode = search_result_mode
188+
self._search_result_mode_lock = threading.Lock()
187189
self._location = location
188190

189191
credentials, _ = google.auth.default()
@@ -216,19 +218,29 @@ def discovery_engine_search(
216218
if mode is not None:
217219
return self._do_search(query, mode)
218220

219-
# Auto-detect: try CHUNKS first, fall back to DOCUMENTS
220-
# if the datastore requires it.
221-
try:
222-
return self._do_search(query, SearchResultMode.CHUNKS)
223-
except GoogleAPICallError as e:
224-
if _STRUCTURED_STORE_ERROR_PATTERN.search(str(e)):
225-
logger.info(
226-
'CHUNKS mode failed for structured datastore,'
227-
' retrying with DOCUMENTS mode.'
228-
)
229-
self._search_result_mode = SearchResultMode.DOCUMENTS
230-
return self._do_search(query, SearchResultMode.DOCUMENTS)
231-
raise
221+
# Auto-detect is per datastore, not per query. Keep the probe
222+
# single-flight so concurrent first calls do not all spend a CHUNKS
223+
# request before learning the same DOCUMENTS fallback.
224+
with self._search_result_mode_lock:
225+
mode = self._search_result_mode
226+
if mode is None:
227+
try:
228+
result = self._do_search(query, SearchResultMode.CHUNKS)
229+
except GoogleAPICallError as e:
230+
if _STRUCTURED_STORE_ERROR_PATTERN.search(str(e)):
231+
logger.info(
232+
'CHUNKS mode failed for structured datastore,'
233+
' retrying with DOCUMENTS mode.'
234+
)
235+
self._search_result_mode = SearchResultMode.DOCUMENTS
236+
mode = SearchResultMode.DOCUMENTS
237+
else:
238+
raise
239+
else:
240+
self._search_result_mode = SearchResultMode.CHUNKS
241+
return result
242+
243+
return self._do_search(query, mode)
232244
except GoogleAPICallError as e:
233245
return {'status': 'error', 'error_message': str(e)}
234246

tests/unittests/tools/test_discovery_engine_search_tool.py

Lines changed: 97 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
@@ -496,6 +499,100 @@ def test_auto_detect_falls_back_to_documents(self, mock_search_client):
496499
# Mode should be persisted so subsequent calls skip the retry.
497500
assert tool._search_result_mode == SearchResultMode.DOCUMENTS
498501

502+
@mock.patch.object(
503+
discoveryengine,
504+
"SearchServiceClient",
505+
)
506+
def test_auto_detect_caches_chunks_on_success(self, mock_search_client):
507+
"""Test auto-detect caches CHUNKS mode on successful search."""
508+
mock_chunk = discoveryengine.Chunk(
509+
document_metadata={
510+
"title": "Jira Issue",
511+
"uri": "https://jira.example.com/123",
512+
"struct_data": {
513+
"summary": "Bug fix",
514+
},
515+
},
516+
content="Bug fix",
517+
)
518+
mock_response = discoveryengine.SearchResponse()
519+
mock_response.results = [
520+
discoveryengine.SearchResponse.SearchResult(chunk=mock_chunk)
521+
]
522+
mock_search_client.return_value.search.return_value = mock_response
523+
524+
tool = DiscoveryEngineSearchTool(data_store_id="test_data_store")
525+
result = tool.discovery_engine_search("test query")
526+
527+
assert result["status"] == "success"
528+
assert len(result["results"]) == 1
529+
assert result["results"][0]["title"] == "Jira Issue"
530+
assert result["results"][0]["url"] == "https://jira.example.com/123"
531+
assert result["results"][0]["content"] == "Bug fix"
532+
assert mock_search_client.return_value.search.call_count == 1
533+
# Mode should be persisted as CHUNKS.
534+
assert tool._search_result_mode == SearchResultMode.CHUNKS
535+
536+
@mock.patch.object(
537+
discoveryengine,
538+
"SearchServiceClient",
539+
)
540+
def test_auto_detect_singleflights_structured_fallback(
541+
self, mock_search_client
542+
):
543+
"""Concurrent cold calls should share one CHUNKS probe."""
544+
spec_cls = discoveryengine.SearchRequest.ContentSearchSpec
545+
worker_count = 8
546+
start_barrier = threading.Barrier(worker_count)
547+
search_lock = threading.Lock()
548+
search_modes = []
549+
structured_error = exceptions.InvalidArgument(
550+
"`content_search_spec.search_result_mode` must be set to"
551+
" SearchRequest.ContentSearchSpec.SearchResultMode.DOCUMENTS"
552+
" when the engine contains structured data store."
553+
)
554+
mock_doc = discoveryengine.Document(
555+
name="projects/p/locations/l/doc1",
556+
id="doc1",
557+
struct_data={
558+
"title": "Jira Issue",
559+
"uri": "https://jira.example.com/123",
560+
"summary": "Bug fix",
561+
},
562+
)
563+
mock_doc_response = discoveryengine.SearchResponse()
564+
mock_doc_response.results = [
565+
discoveryengine.SearchResponse.SearchResult(document=mock_doc)
566+
]
567+
568+
def search(request):
569+
mode = request.content_search_spec.search_result_mode
570+
with search_lock:
571+
search_modes.append(mode)
572+
if mode == spec_cls.SearchResultMode.CHUNKS:
573+
time.sleep(0.05)
574+
raise structured_error
575+
return mock_doc_response
576+
577+
mock_search_client.return_value.search.side_effect = search
578+
tool = DiscoveryEngineSearchTool(data_store_id="test_data_store")
579+
580+
def run_search(index):
581+
start_barrier.wait(timeout=5)
582+
return tool.discovery_engine_search(f"test query {index}")
583+
584+
with concurrent.futures.ThreadPoolExecutor(
585+
max_workers=worker_count
586+
) as executor:
587+
results = list(executor.map(run_search, range(worker_count)))
588+
589+
assert all(result["status"] == "success" for result in results)
590+
assert search_modes.count(spec_cls.SearchResultMode.CHUNKS) == 1
591+
assert (
592+
search_modes.count(spec_cls.SearchResultMode.DOCUMENTS) == worker_count
593+
)
594+
assert tool._search_result_mode == SearchResultMode.DOCUMENTS
595+
499596
@mock.patch.object(
500597
discoveryengine,
501598
"SearchServiceClient",

0 commit comments

Comments
 (0)