mirror of
https://github.com/dograh-hq/dograh.git
synced 2026-06-07 07:55:16 +02:00
fix(telephony): resolve transfer context via call-sid index instead of KEYS scan (#387)
Transfer-context lookup by original_call_sid ran
`redis.keys("transfer:context:*")` and iterated every match — an O(N)
blocking scan on call-control hot paths, duplicated across the ARI
manager and the Twilio/Telnyx conference strategies.
Maintain a `transfer:by_call_sid:{original_call_sid}` -> transfer_id
secondary index, written and cleared alongside the context in
store/remove, and resolve lookups with a direct GET. Route the
Twilio/Telnyx strategies through the manager so the lookup lives in one
place (also dropping per-call ad-hoc Redis connections).
Closes #328
Co-authored-by: shiminshen <16914659+shiminshen@users.noreply.github.com>
This commit is contained in:
parent
7ba95c0fbe
commit
37e7f4d2e6
5 changed files with 160 additions and 53 deletions
|
|
@ -47,6 +47,11 @@ class CallTransferManager:
|
|||
redis = await self._get_redis()
|
||||
key = TransferRedisChannels.transfer_context_key(context.transfer_id)
|
||||
await redis.setex(key, ttl, context.to_json())
|
||||
if context.original_call_sid:
|
||||
index_key = TransferRedisChannels.transfer_context_by_call_sid_key(
|
||||
context.original_call_sid
|
||||
)
|
||||
await redis.setex(index_key, ttl, context.transfer_id)
|
||||
logger.debug(f"Stored transfer context for {context.transfer_id}")
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to store transfer context: {e}")
|
||||
|
|
@ -79,8 +84,15 @@ class CallTransferManager:
|
|||
"""
|
||||
try:
|
||||
redis = await self._get_redis()
|
||||
context = await self.get_transfer_context(transfer_id)
|
||||
key = TransferRedisChannels.transfer_context_key(transfer_id)
|
||||
await redis.delete(key)
|
||||
if context and context.original_call_sid:
|
||||
index_key = TransferRedisChannels.transfer_context_by_call_sid_key(
|
||||
context.original_call_sid
|
||||
)
|
||||
await redis.delete(key, index_key)
|
||||
else:
|
||||
await redis.delete(key)
|
||||
logger.debug(f"Removed transfer context for {transfer_id}")
|
||||
except Exception as e:
|
||||
logger.error(f"Failed to remove transfer context: {e}")
|
||||
|
|
@ -186,24 +198,24 @@ class CallTransferManager:
|
|||
logger.error(f"Error closing pubsub connection: {e}")
|
||||
|
||||
async def find_transfer_context_for_call(self, caller_channel_id: str):
|
||||
"""Find the active transfer context for this caller channel."""
|
||||
|
||||
redis = await self._get_redis()
|
||||
"""Find the active transfer context for this caller channel.
|
||||
|
||||
Resolves via the original_call_sid -> transfer_id secondary index
|
||||
(see store_transfer_context) instead of scanning the keyspace with
|
||||
``KEYS transfer:context:*``.
|
||||
"""
|
||||
try:
|
||||
# Search Redis for transfer contexts where original_call_sid matches this caller
|
||||
transfer_keys = await redis.keys("transfer:context:*")
|
||||
|
||||
for key in transfer_keys:
|
||||
try:
|
||||
context_data = await redis.get(key)
|
||||
if context_data:
|
||||
context = TransferContext.from_json(context_data)
|
||||
if context.original_call_sid == caller_channel_id:
|
||||
return context
|
||||
except Exception:
|
||||
continue
|
||||
redis = await self._get_redis()
|
||||
index_key = TransferRedisChannels.transfer_context_by_call_sid_key(
|
||||
caller_channel_id
|
||||
)
|
||||
transfer_id = await redis.get(index_key)
|
||||
if not transfer_id:
|
||||
return None
|
||||
|
||||
context = await self.get_transfer_context(transfer_id)
|
||||
if context and context.original_call_sid == caller_channel_id:
|
||||
return context
|
||||
return None
|
||||
|
||||
except Exception as e:
|
||||
|
|
|
|||
|
|
@ -116,25 +116,14 @@ class TelnyxConferenceStrategy(TransferStrategy):
|
|||
async def _find_transfer_context_for_call(self, caller_call_control_id: str):
|
||||
"""Find the active transfer context whose original_call_sid matches."""
|
||||
try:
|
||||
import redis.asyncio as aioredis
|
||||
from api.services.telephony.call_transfer_manager import (
|
||||
get_call_transfer_manager,
|
||||
)
|
||||
|
||||
from api.constants import REDIS_URL
|
||||
from api.services.telephony.transfer_event_protocol import TransferContext
|
||||
|
||||
redis = aioredis.from_url(REDIS_URL, decode_responses=True)
|
||||
transfer_keys = await redis.keys("transfer:context:*")
|
||||
|
||||
for key in transfer_keys:
|
||||
try:
|
||||
context_data = await redis.get(key)
|
||||
if context_data:
|
||||
context = TransferContext.from_json(context_data)
|
||||
if context.original_call_sid == caller_call_control_id:
|
||||
return context
|
||||
except Exception:
|
||||
continue
|
||||
|
||||
return None
|
||||
manager = await get_call_transfer_manager()
|
||||
return await manager.find_transfer_context_for_call(
|
||||
caller_call_control_id
|
||||
)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(f"[Telnyx Transfer] Error finding transfer context: {e}")
|
||||
|
|
|
|||
|
|
@ -106,26 +106,12 @@ class TwilioConferenceStrategy(TransferStrategy):
|
|||
async def _find_transfer_context_for_call(self, call_sid: str):
|
||||
"""Find the active transfer context for this call."""
|
||||
try:
|
||||
import redis.asyncio as aioredis
|
||||
from api.services.telephony.call_transfer_manager import (
|
||||
get_call_transfer_manager,
|
||||
)
|
||||
|
||||
from api.constants import REDIS_URL
|
||||
from api.services.telephony.transfer_event_protocol import TransferContext
|
||||
|
||||
# Search Redis for transfer contexts where original_call_sid matches
|
||||
redis = aioredis.from_url(REDIS_URL, decode_responses=True)
|
||||
transfer_keys = await redis.keys("transfer:context:*")
|
||||
|
||||
for key in transfer_keys:
|
||||
try:
|
||||
context_data = await redis.get(key)
|
||||
if context_data:
|
||||
context = TransferContext.from_json(context_data)
|
||||
if context.original_call_sid == call_sid:
|
||||
return context
|
||||
except Exception:
|
||||
continue
|
||||
|
||||
return None
|
||||
call_transfer_manager = await get_call_transfer_manager()
|
||||
return await call_transfer_manager.find_transfer_context_for_call(call_sid)
|
||||
|
||||
except Exception as e:
|
||||
logger.error(f"[Twilio Transfer] Error finding transfer context: {e}")
|
||||
|
|
|
|||
|
|
@ -99,3 +99,12 @@ class TransferRedisChannels:
|
|||
def transfer_context_key(transfer_id: str) -> str:
|
||||
"""Redis key for transfer context storage."""
|
||||
return f"transfer:context:{transfer_id}"
|
||||
|
||||
@staticmethod
|
||||
def transfer_context_by_call_sid_key(original_call_sid: str) -> str:
|
||||
"""Redis key for the original_call_sid -> transfer_id secondary index.
|
||||
|
||||
Lets a caller's transfer context be resolved with a direct lookup
|
||||
instead of an O(N) ``KEYS transfer:context:*`` keyspace scan.
|
||||
"""
|
||||
return f"transfer:by_call_sid:{original_call_sid}"
|
||||
|
|
|
|||
111
api/tests/telephony/test_call_transfer_manager.py
Normal file
111
api/tests/telephony/test_call_transfer_manager.py
Normal file
|
|
@ -0,0 +1,111 @@
|
|||
"""Tests for CallTransferManager Redis-backed transfer-context lookup.
|
||||
|
||||
These tests verify (regression for issue #328):
|
||||
1. Lookup by original_call_sid resolves via a secondary index, never an
|
||||
O(N) `KEYS transfer:context:*` keyspace scan.
|
||||
2. A lookup for an unknown call sid returns None without scanning.
|
||||
3. Removing a transfer context also clears its call-sid index entry.
|
||||
"""
|
||||
|
||||
from typing import Dict, List
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
class _FakeRedis:
|
||||
"""Minimal in-memory async Redis double.
|
||||
|
||||
Counts calls to ``keys()`` so tests can assert the lookup path no longer
|
||||
performs an O(N) keyspace scan (the regression behind issue #328).
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self._store: Dict[str, str] = {}
|
||||
self.keys_call_count = 0
|
||||
|
||||
async def setex(self, key: str, ttl: int, value: str) -> None:
|
||||
self._store[key] = value
|
||||
|
||||
async def get(self, key: str):
|
||||
return self._store.get(key)
|
||||
|
||||
async def delete(self, *keys: str) -> None:
|
||||
for key in keys:
|
||||
self._store.pop(key, None)
|
||||
|
||||
async def keys(self, pattern: str) -> List[str]:
|
||||
self.keys_call_count += 1
|
||||
if pattern.endswith("*"):
|
||||
prefix = pattern[:-1]
|
||||
return [k for k in self._store if k.startswith(prefix)]
|
||||
return [k for k in self._store if k == pattern]
|
||||
|
||||
|
||||
def _build_context(transfer_id: str, original_call_sid: str):
|
||||
from api.services.telephony.transfer_event_protocol import TransferContext
|
||||
|
||||
return TransferContext(
|
||||
transfer_id=transfer_id,
|
||||
call_sid="dest-call-sid",
|
||||
target_number="+15551230000",
|
||||
tool_uuid="tool-uuid",
|
||||
original_call_sid=original_call_sid,
|
||||
conference_name="conference-name",
|
||||
initiated_at=0.0,
|
||||
)
|
||||
|
||||
|
||||
class TestFindTransferContextByCallSid:
|
||||
"""Lookup must use the call-sid index, not a KEYS scan (issue #328)."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_lookup_uses_index_and_not_keys_scan(self):
|
||||
from api.services.telephony.call_transfer_manager import CallTransferManager
|
||||
|
||||
fake = _FakeRedis()
|
||||
manager = CallTransferManager(redis_client=fake)
|
||||
|
||||
await manager.store_transfer_context(
|
||||
_build_context("tx-1", "caller-abc")
|
||||
)
|
||||
|
||||
found = await manager.find_transfer_context_for_call("caller-abc")
|
||||
|
||||
assert found is not None
|
||||
assert found.transfer_id == "tx-1"
|
||||
# Regression (issue #328): the lookup must resolve via the secondary
|
||||
# index, never an O(N) `KEYS transfer:context:*` keyspace scan.
|
||||
assert fake.keys_call_count == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_lookup_returns_none_for_unknown_call_sid(self):
|
||||
from api.services.telephony.call_transfer_manager import CallTransferManager
|
||||
|
||||
fake = _FakeRedis()
|
||||
manager = CallTransferManager(redis_client=fake)
|
||||
|
||||
await manager.store_transfer_context(
|
||||
_build_context("tx-1", "caller-abc")
|
||||
)
|
||||
|
||||
found = await manager.find_transfer_context_for_call("not-a-caller")
|
||||
|
||||
assert found is None
|
||||
assert fake.keys_call_count == 0
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_remove_clears_call_sid_index(self):
|
||||
from api.services.telephony.call_transfer_manager import CallTransferManager
|
||||
|
||||
fake = _FakeRedis()
|
||||
manager = CallTransferManager(redis_client=fake)
|
||||
|
||||
await manager.store_transfer_context(
|
||||
_build_context("tx-1", "caller-abc")
|
||||
)
|
||||
await manager.remove_transfer_context("tx-1")
|
||||
|
||||
found = await manager.find_transfer_context_for_call("caller-abc")
|
||||
|
||||
assert found is None
|
||||
assert fake.keys_call_count == 0
|
||||
Loading…
Add table
Add a link
Reference in a new issue