Skip to content

Commit c9102a9

Browse files
grayhempSergey Konoplevtheagenticguy
authored
feat(redshift-mcp-server): long poll the Data API for statement status (#4560)
* feat(redshift-mcp-server): long poll the Data API for statement status Every statement waited for completion by calling DescribeStatement once per second, and each execute_query runs three statements (BEGIN READ ONLY, user SQL, ROLLBACK), so a warm read-only query cost at least 8 Data API calls: 3 ExecuteStatement, 4 DescribeStatement and 1 GetStatementResult. The Data API now supports long polling. WaitTimeSeconds (1-30) holds the request open until the statement reaches a terminal status or the wait elapses, and is accepted on both ExecuteStatement and DescribeStatement. - Send WaitTimeSeconds, from the new QUERY_LONG_POLL constant, on ExecuteStatement and on every DescribeStatement poll. Setting it to 0 restores plain QUERY_POLL_INTERVAL polling. - Collapse the completion wait into a single loop that evaluates whichever response came last, since both operations report Status identically. - Return the terminal statement response from _execute_statement rather than just the id, so _create_session_with_app_name reads SessionId and _execute_protected_statement reads HasResultSet off it. Both callers drop their own DescribeStatement call. - Track the timeout with time.monotonic(), since accumulated sleeps stop approximating elapsed time once a call blocks server-side. - Fall back to plain polling on ActiveWaitingRequestsExceededException, which the service raises when too many requests wait on one statement. - Require boto3/botocore >= 1.43.55, the first release carrying WaitTimeSeconds in the redshift-data service model. - Cover long polling, the terminal-status shortcut, the disabled path and the waiter-limit fallback in TestExecuteStatement. A warm read-only query now costs 4 calls, and a statement with no result set costs 1. Only a failing statement still needs a DescribeStatement, to read the Error field that ExecuteStatement does not carry. Verified end-to-end against live provisioned and serverless clusters. * fix(redshift-mcp-server): run Data API calls off the event loop boto3 is synchronous, so long polling turned each Data API call into an event loop stall of up to WaitTimeSeconds. Measured against a live cluster, a 79s query blocked the loop for 48s in one stretch and let a 50ms heartbeat tick once rather than the ~1579 times expected, which serialized every other in-flight tool call and stalled MCP protocol traffic. Before long polling the same calls blocked only for a short HTTP round trip. - Run execute_statement and both describe_statement calls in _execute_statement through asyncio.to_thread. - Add a regression test that fails when a blocking Data API call starves a concurrent coroutine. The same query now yields 1552 heartbeat ticks with a 0.05s worst gap. Reported by Copilot on #4560. --------- Co-authored-by: Sergey Konoplev <sergkono@amazon.com> Co-authored-by: Laith Al-Saadoon <9553966+theagenticguy@users.noreply.github.com>
1 parent 6d6067c commit c9102a9

5 files changed

Lines changed: 334 additions & 131 deletions

File tree

src/redshift-mcp-server/awslabs/redshift_mcp_server/consts.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
DEFAULT_LOG_LEVEL = 'WARNING'
2323
QUERY_TIMEOUT = 3600
2424
QUERY_POLL_INTERVAL = 1
25+
QUERY_LONG_POLL = 30
2526
SESSION_KEEPALIVE = 600
2627

2728
# SQL discovery commands. Results are read positionally; {placeholders} are

src/redshift-mcp-server/awslabs/redshift_mcp_server/redshift.py

Lines changed: 52 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
CLIENT_USER_AGENT_NAME,
2727
COLUMNS_SQL,
2828
DATABASES_SQL,
29+
QUERY_LONG_POLL,
2930
QUERY_POLL_INTERVAL,
3031
QUERY_TIMEOUT,
3132
SCHEMAS_SQL,
@@ -206,18 +207,15 @@ async def _create_session_with_app_name(
206207
app_name_sql = f"SET application_name TO '{self._app_name}';"
207208

208209
# Execute statement to create session
209-
statement_id = await _execute_statement(
210+
response = await _execute_statement(
210211
cluster_info=cluster_info,
211212
cluster_identifier=cluster_identifier,
212213
database_name=database_name,
213214
sql=app_name_sql,
214215
session_keepalive=self._session_keepalive,
215216
)
216217

217-
# Get session ID from the response
218-
data_client = client_manager.redshift_data_client()
219-
status_response = data_client.describe_statement(Id=statement_id)
220-
session_id = status_response['SessionId']
218+
session_id = response['SessionId']
221219

222220
logger.debug(f'Created session with application name: {session_id}')
223221
return session_id
@@ -296,7 +294,7 @@ async def _execute_protected_statement(
296294
if allow_read_write:
297295
# Read-write: run the single guarded statement directly (autocommit). No
298296
# transaction wrapper. Any error propagates.
299-
user_query_id = await _execute_statement(
297+
user_statement = await _execute_statement(
300298
cluster_info=cluster_info,
301299
cluster_identifier=cluster_identifier,
302300
database_name=database_name,
@@ -316,11 +314,11 @@ async def _execute_protected_statement(
316314
)
317315

318316
# Execute user SQL with parameters, ensuring the transaction is always closed.
319-
user_query_id = None
317+
user_statement = None
320318
user_sql_error: Exception | None = None
321319

322320
try:
323-
user_query_id = await _execute_statement(
321+
user_statement = await _execute_statement(
324322
cluster_info=cluster_info,
325323
cluster_identifier=cluster_identifier,
326324
database_name=database_name,
@@ -357,14 +355,14 @@ async def _execute_protected_statement(
357355
raise user_sql_error
358356

359357
# Get results from user query (shared by both modes); runs outside the lock.
360-
# describe_statement / get_statement_result are keyed by query_id, not session-bound,
361-
# so the lock is not held during the (potentially unbounded) results wait.
362-
data_client = client_manager.redshift_data_client()
363-
assert user_query_id is not None, 'user_query_id should not be None at this point'
358+
# get_statement_result is keyed by query_id, not session-bound, so the lock is not
359+
# held during the (potentially unbounded) results wait.
360+
assert user_statement is not None, 'user_statement should not be None at this point'
361+
user_query_id = user_statement['Id']
364362

365363
# Only fetch results when the statement produced a result set (e.g. SET does not).
366-
describe_response = data_client.describe_statement(Id=user_query_id)
367-
if describe_response.get('HasResultSet'):
364+
if user_statement.get('HasResultSet'):
365+
data_client = client_manager.redshift_data_client()
368366
results_response = data_client.get_statement_result(Id=user_query_id)
369367
else:
370368
results_response = {'Records': [], 'ColumnMetadata': []}
@@ -381,7 +379,8 @@ async def _execute_statement(
381379
session_keepalive: int | None = None,
382380
query_poll_interval: float = QUERY_POLL_INTERVAL,
383381
query_timeout: float = QUERY_TIMEOUT,
384-
) -> str:
382+
query_long_poll: int = QUERY_LONG_POLL,
383+
) -> dict:
385384
"""Execute a single statement with optional session support and parameters.
386385
387386
Args:
@@ -394,9 +393,13 @@ async def _execute_statement(
394393
session_keepalive: Optional session keepalive seconds (only used when session_id is None).
395394
query_poll_interval: Polling interval in seconds for checking query status.
396395
query_timeout: Maximum time in seconds to wait for query completion.
396+
query_long_poll: Data API WaitTimeSeconds, 1-30, or 0 to disable long polling.
397397
398398
Returns:
399-
Statement ID from the ExecuteStatement response.
399+
The terminal statement response, carrying Id, Status, SessionId and HasResultSet.
400+
401+
Raises:
402+
Exception: If the statement fails, is aborted, or times out.
400403
"""
401404
data_client = client_manager.redshift_data_client()
402405

@@ -423,35 +426,54 @@ async def _execute_statement(
423426
elif session_keepalive is not None:
424427
request_params['SessionKeepAliveSeconds'] = session_keepalive
425428

426-
response = data_client.execute_statement(**request_params)
429+
long_poll_params = {'WaitTimeSeconds': query_long_poll} if query_long_poll else {}
430+
431+
# boto3 is synchronous and a long poll holds the caller for up to query_long_poll
432+
# seconds, so every Data API call here runs off the event loop.
433+
response = await asyncio.to_thread(
434+
data_client.execute_statement, **request_params, **long_poll_params
435+
)
427436
statement_id = response['Id']
428437

429438
logger.debug(
430439
f'Executed statement: {statement_id}' + (f' in session {session_id}' if session_id else '')
431440
)
432441

433-
# Wait for statement completion
434-
wait_time = 0
435-
while wait_time < query_timeout:
436-
status_response = data_client.describe_statement(Id=statement_id)
437-
status = status_response['Status']
442+
# ExecuteStatement and DescribeStatement report status alike, so one loop settles the
443+
# long-polled submit and every later poll. Wall clock, since a long poll blocks server-side.
444+
deadline = time.monotonic() + query_timeout
445+
while True:
446+
status = response.get('Status')
438447

439448
if status == 'FINISHED':
440449
logger.debug(f'Statement completed: {statement_id}')
441-
break
450+
return response
442451
elif status in ['FAILED', 'ABORTED']:
443-
error_msg = status_response.get('Error', 'Unknown error')
452+
error_msg = response.get('Error')
453+
if error_msg is None:
454+
# ExecuteStatement carries no Error field, so the reason takes a describe.
455+
described = await asyncio.to_thread(
456+
data_client.describe_statement, Id=statement_id
457+
)
458+
error_msg = described.get('Error', 'Unknown error')
444459
logger.error(f'Statement failed: {error_msg}')
445460
raise Exception(f'Statement failed: {error_msg}')
446461

447-
await asyncio.sleep(query_poll_interval)
448-
wait_time += query_poll_interval
462+
if time.monotonic() >= deadline:
463+
logger.error(f'Statement timed out: {statement_id}')
464+
raise Exception(f'Statement timed out after {query_timeout} seconds')
449465

450-
if wait_time >= query_timeout:
451-
logger.error(f'Statement timed out: {statement_id}')
452-
raise Exception(f'Statement timed out after {wait_time} seconds')
466+
await asyncio.sleep(query_poll_interval)
453467

454-
return statement_id
468+
try:
469+
response = await asyncio.to_thread(
470+
data_client.describe_statement, Id=statement_id, **long_poll_params
471+
)
472+
except ClientError as e:
473+
if e.response.get('Error', {}).get('Code') != 'ActiveWaitingRequestsExceededException':
474+
raise
475+
logger.warning(f'Long polling limit reached, polling instead: {statement_id}')
476+
long_poll_params = {}
455477

456478

457479
async def discover_clusters() -> list[RedshiftCluster]:

src/redshift-mcp-server/pyproject.toml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ description = "An AWS Labs Model Context Protocol (MCP) server for Redshift"
88
readme = "README.md"
99
requires-python = ">=3.10"
1010
dependencies = [
11-
"boto3>=1.38.39",
12-
"botocore>=1.38.39",
11+
# Data API long polling (WaitTimeSeconds) entered the service model in 1.43.55.
12+
"boto3>=1.43.55",
13+
"botocore>=1.43.55",
1314
"loguru>=0.7.0",
1415
"mcp[cli]>=2.0.0,<3.0.0",
1516
"pydantic>=2.10.6",

0 commit comments

Comments
 (0)