Feature : 분산 시스템 세션 개선 - Redis Pub/sub #23
Conversation
Walkthrough이 PR은 분산 메시지 브로커(로컬/Redis Pub/Sub) 도입, 세션/웹소켓 관리 계층 분리, 서버 등록·하트비트 인프라 추가, 구성값(appsettings/.env/docker-compose) 확장, 기존 IWebSocketManager 제거 및 메시지 브로커 기반으로 채팅 파이프라인을 전환하고, 관련 테스트/문서/스크립트를 추가·갱신합니다. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as WebSocket Client
participant API as API Server
participant MW as WebSocketMiddleware
participant SM as ISessionManager
participant WCM as IWebSocketConnectionManager
participant SRS as IServerRegistrationService
Client->>API: HTTP Upgrade (JWT)
API->>MW: Invoke
MW->>SM: CreateSessionAsync(userId)
MW->>WCM: RegisterConnection(sessionId, ws)
alt 서버 등록 서비스 사용 시
MW->>SRS: SetUserServerAsync(userId, serverId)
end
loop 메시지 수신
Client-->>MW: ping
MW->>Client: pong
par 비동기 하트비트
MW->>SM: UpdateSessionHeartbeatAsync(userId)
end
end
Client-x MW: Disconnect/Close
MW->>WCM: UnregisterConnection(sessionId)
MW->>SM: DeleteSessionAsync(userId)
alt 서버 등록 서비스 사용 시
MW->>SRS: RemoveUserServerAsync(userId)
end
sequenceDiagram
participant Producer as Chat Handler/Processor
participant MB as IMessageBroker
participant LCM as IWebSocketConnectionManager
participant SRS as IServerRegistrationService
participant Redis as Redis Pub/Sub
participant API2 as Target API Server
Producer->>MB: SendToUserAsync(userId, message)
alt 로컬 연결 존재
MB->>LCM: SendText/Binary(sessionId, payload)
else 원격 서버
MB->>SRS: GetUserServerAsync(userId)
MB->>Redis: PUBLISH user:{userId} payload
Redis-->>API2: SUBSCRIBE user:{userId} delivery
API2->>LCM: SendText/Binary(sessionId, payload)
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
test-clients/ai-chat-client/script.js (2)
4-6: 포트 감지 로직 버그: 항상 7900으로 고정됩니다.주석과 달리 개발/기본 포트 선택이 항상 7900으로 평가됩니다. 로컬에서 다른 포트(예: 7910/7901)로 띄우면 연결 실패 가능성이 큽니다.
아래처럼 현재 페이지 포트 또는 기본값을 사용하고, 환경 표시는 호스트 기준으로 분기하는 쪽이 안전합니다.
-const isDevelopment = currentPort === '7900'; -const serverPort = isDevelopment ? '7900' : '7900'; +const isDevelopment = (window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1'); +const serverPort = currentPort || '7900';또한 혼합 콘텐츠를 피하려면 프로토콜도 동적으로 설정하세요:
-const WS_URL = `ws://${ENDPOINT}/ws`; -const HTTP_URL = `http://${ENDPOINT}/api/v1/chat`; -const LOGIN_URL = `http://${ENDPOINT}/api/v1/auth/guest-login`; -const CHARACTER_BASE_URL = `http://${ENDPOINT}/api/v1/character`; -const CONVERSATION_BASE_URL = `http://${ENDPOINT}/api/v1/conversation`; -const TOKEN_BASE_URL = `http://${ENDPOINT}/api/v1/credits`; +const HTTP_SCHEME = window.location.protocol === 'https:' ? 'https' : 'http'; +const WS_SCHEME = window.location.protocol === 'https:' ? 'wss' : 'ws'; +const WS_URL = `${WS_SCHEME}://${ENDPOINT}/ws`; +const HTTP_URL = `${HTTP_SCHEME}://${ENDPOINT}/api/v1/chat`; +const LOGIN_URL = `${HTTP_SCHEME}://${ENDPOINT}/api/v1/auth/guest-login`; +const CHARACTER_BASE_URL = `${HTTP_SCHEME}://${ENDPOINT}/api/v1/character`; +const CONVERSATION_BASE_URL = `${HTTP_SCHEME}://${ENDPOINT}/api/v1/conversation`; +const TOKEN_BASE_URL = `${HTTP_SCHEME}://${ENDPOINT}/api/v1/credits`;
197-242: ReferenceError: parseBinaryMessage가 정의되지 않은 sessionId를 반환합니다.객체 리터럴에 sessionId 변수를 사용하지만, 어디에도 정의/파싱되지 않습니다. 런타임 ReferenceError로 null 반환만 유발합니다.
- return { - sessionId, - text, - audioData, - audioLength: audioDuration - }; + return { + text, + audioData, + audioLength: audioDuration + };추가로, 바이너리 핸들러의 sessionId 업데이트 블록도 제거하거나 실제로 파싱하도록 맞춰주세요(아래 관련 코멘트 참조).
🧹 Nitpick comments (49)
scripts/start-server-3.ps1 (1)
16-17: 스크립트 실행 위치에 독립적인 프로젝트 경로 사용 제안현재 상대 경로에 의존합니다. 어떤 위치에서 실행해도 동작하도록
$PSScriptRoot를 사용하세요.아래 변경을 제안합니다:
Write-Host "`n📡 서버 3 시작 중..." -dotnet run --project ProjectVG.Api --urls "http://localhost:7912" + $projectPath = (Resolve-Path (Join-Path $PSScriptRoot "..\ProjectVG.Api")).Path + dotnet run --project $projectPath --urls "http://localhost:7912"scripts/start-server-2.ps1 (1)
16-17: 스크립트 실행 위치에 독립적인 프로젝트 경로 사용 제안server-3 스크립트와 동일하게
$PSScriptRoot를 사용해 견고성을 높이세요.Write-Host "`n📡 서버 2 시작 중..." -dotnet run --project ProjectVG.Api --urls "http://localhost:7911" + $projectPath = (Resolve-Path (Join-Path $PSScriptRoot "..\ProjectVG.Api")).Path + dotnet run --project $projectPath --urls "http://localhost:7911".env.example (1)
6-11: ENVIRONMENT와 ASPNETCORE_ENVIRONMENT 불일치예시 템플릿에서 서로 다른 값을 사용하면 혼란을 유발할 수 있습니다. 기본값을 일치시키는 것을 권장합니다.
-ASPNETCORE_ENVIRONMENT=Production +ASPNETCORE_ENVIRONMENT=DevelopmentAlso applies to: 45-46
scripts/test-distributed-system.ps1 (1)
49-49: 스크립트 실행 위치에 독립적인 프로젝트 경로 사용 제안다른 시작 스크립트와 동일하게
$PSScriptRoot기반 경로를 사용하면 신뢰성이 높아집니다.-dotnet run --project ProjectVG.Api --urls "http://localhost:7910" + $projectPath = (Resolve-Path (Join-Path $PSScriptRoot "..\ProjectVG.Api")).Path + dotnet run --project $projectPath --urls "http://localhost:7910"ProjectVG.Application/Services/MessageBroker/IMessageBroker.cs (2)
1-1: 취소 토큰 지원으로 API 탄력성 향상 제안비동기 작업에
CancellationToken을 추가하면 요청 취소, 종료 시그널, 타임아웃 처리에 유리합니다.+using System.Threading; +using System.Threading.Tasks; namespace ProjectVG.Application.Services.MessageBroker { public interface IMessageBroker { /// <summary> /// 특정 사용자에게 메시지를 전송합니다 /// </summary> - Task SendToUserAsync(string userId, object message); + Task SendToUserAsync(string userId, object message, CancellationToken cancellationToken = default); /// <summary> /// 모든 연결된 사용자에게 메시지를 방송합니다 /// </summary> - Task BroadcastAsync(object message); + Task BroadcastAsync(object message, CancellationToken cancellationToken = default); /// <summary> /// 특정 서버로 메시지를 전송합니다 /// </summary> - Task SendToServerAsync(string serverId, object message); + Task SendToServerAsync(string serverId, object message, CancellationToken cancellationToken = default); /// <summary> /// 메시지 브로커가 분산 모드인지 확인합니다 /// </summary> bool IsDistributed { get; } } }Also applies to: 8-8, 13-13, 18-18
5-24: 메시지 형식의 강한 타이핑 고려(권장)
object는 직렬화/검증에서 타입 안정성을 잃습니다.BrokerMessage(이미 도입됐다면) 또는 제네릭IMessageBroker<TMessage>채택을 검토해 보세요.test-clients/ai-chat-client/script.js (5)
114-119: serverInfo 요소 null 가드와 라벨 조건부 표시 필요DOM에 server-info가 없으면 즉시 예외가 납니다. 또 항상 “(디버그 모드)”로 표시됩니다.
-function updateServerInfo() { - serverInfo.textContent = `${ENDPOINT} (디버그 모드)`; -} +function updateServerInfo() { + if (!serverInfo) return; + serverInfo.textContent = `${ENDPOINT}${isDevelopment ? ' (디버그 모드)' : ''}`; +}
491-496: 정의되지 않은 전역 변수 sessionId 사용여기서 sessionId를 대입/사용하지만 선언되지 않았습니다. 위의 parseBinaryMessage 수정과 함께 이 블록을 제거하는 것이 일관됩니다.
- // 세션 ID 업데이트 - if (result.sessionId) { - sessionId = result.sessionId; - updateSessionId(sessionId); - } + // (세션 ID는 바이너리 페이로드에 포함되지 않으므로 처리 생략)만약 세션 ID를 바이너리로 지원하려면, parseBinaryMessage에서 실제로 sessionId를 읽어 반환하도록 스펙과 구현을 함께 변경하세요.
306-307: 오디오 URL 해제 누락: 메모리 누수 가능Object URL을 revoke하지 않아 누수가 발생할 수 있습니다.
-audioPlayer.onended = () => playNextAudio(); +audioPlayer.onended = () => { + try { URL.revokeObjectURL(audioPlayer.src); } catch {} + playNextAudio(); +};
308-311: appendLog의 innerHTML 사용은 XSS 위험서버에서 온 텍스트를 그대로 innerHTML에 넣으면 위험합니다. 동적 값에는 escapeHtml을 적용하거나 textContent 기반 렌더링으로 분리하세요.
583-589: 오류/성공 응답 로그는 좋지만, 메시지 오탈자 및 JSON 파싱 예외 주의
- 문자열
❌ HTTP 오류]에 대괄호가 하나 더 들어가 있습니다.- body가 비어있는 경우
res.json()가 예외를 던질 수 있습니다.- appendLog(`<span style='color:red'>[${responseTimestamp}] ❌ HTTP 오류] 상태코드: ${res.status}</span>`); + appendLog(`<span style='color:red'>[${responseTimestamp}] ❌ HTTP 오류: 상태코드 ${res.status}</span>`);선택사항: 안전 파싱
// then 블록에서 return res.text().then(t => t ? JSON.parse(t) : null);docker-compose.yml (2)
8-15: 로그 볼륨 바인드 구성 확인 필요named volume + driver_opts(bind) 패턴은 호스트 경로가 절대경로여야 안정적입니다. ${DATA_PATH}가 상대경로로 확장될 수 있으니 실제 배포 경로를 확인하세요.
- device: ${DATA_PATH:-../data}/logs/api-server + device: ${DATA_PATH:-/var/lib/projectvg}/logs/api-server(예시) 절대 경로 사용 권장
63-67: 로그 로테이션 설정 LGTM + 헬스체크 제안json-file + 로테이션 기본값 적절합니다. 추가로 서비스 헬스체크를 권장합니다.
restart: unless-stopped extra_hosts: - "host.docker.internal:host-gateway" + healthcheck: + test: ["CMD-SHELL", "wget -qO- http://localhost:7900/health || exit 1"] + interval: 30s + timeout: 5s + retries: 3(이미지 내 wget/curl 유무에 맞춰 조정)
scripts/monitor-redis.ps1 (4)
36-47: KEYS 사용은 운영 Redis에서 위험합니다 → SCAN으로 변경 권장KEYS는 O(N)이라 대규모 키에서 블로킹됩니다. SCAN으로 대체하세요.
- $userSessions = redis-cli -p 6380 KEYS "user:server:*" - if ($userSessions) { - foreach ($session in $userSessions) { + $userSessions = redis-cli -p 6380 --scan --pattern "user:server:*" + if ($userSessions) { + foreach ($session in $userSessions) { if ($session) { $userId = $session -replace "user:server:", "" $serverId = redis-cli -p 6380 GET $session Write-Host " 👤 사용자 $userId -> 서버 $serverId" -ForegroundColor Yellow } } } else {
51-57: PUBSUB NUMSUB 출력 파싱 개선NUMSUB 결과는 라인 2가 숫자이므로 추출을 명시하세요.
- $subscribers = redis-cli -p 6380 PUBSUB NUMSUB $channel - Write-Host " 📻 $channel (구독자: $($subscribers[1]))" -ForegroundColor Cyan + $subscribersRaw = redis-cli -p 6380 --raw PUBSUB NUMSUB $channel + $subscriberCount = ($subscribersRaw | Select-Object -Last 1) + Write-Host " 📻 $channel (구독자: $subscriberCount)" -ForegroundColor Cyan
63-68: 메모리 정보 파싱 견고성Split 기반 파싱은 공백/형식 변화에 취약합니다. 정규식/--raw 사용을 권장합니다.
- $memoryInfo = redis-cli -p 6380 INFO memory - $usedMemory = ($memoryInfo | Select-String "used_memory_human:").ToString().Split(":")[1] + $memoryInfo = redis-cli -p 6380 --raw INFO memory + $usedMemory = ($memoryInfo | Select-String "^used_memory_human:").ToString().Split(":")[1].Trim()
1-5: 호스트/포트 파라미터화 제안로컬 고정(6380) 대신 파라미터로 제어하면 재사용성이 좋아집니다.
다음 블록을 파일 상단에 추가하고 모든 redis-cli 호출을
$Port로 치환하십시오.param( [string]$Host = "127.0.0.1", [int]$Port = 6380 )그리고
redis-cli -p 6380 …→redis-cli -h $Host -p $Port …docs/distributed-system/README.md (4)
10-13: 코드 블록 언어 지정 누락(MD040)markdownlint 지적대로 언어를 지정하세요.
-``` +```text Unity Client ──WebSocket──→ API Server ──→ ChatService ──→ WebSocketManager--- `61-70`: **환경변수 블록에 언어 지정** bash로 명시하세요. ```diff -``` +```bash # 분산 모드 활성화 DISTRIBUTED_MODE=true ...
73-83: appsettings.json 블록에 언어 지정json으로 명시하세요.
-``` +```json { "DistributedSystem": { "Enabled": true, ... } }
176-187: WebSocket 예시 코드 블록에 언어 지정javascript로 명시하세요.
-``` +```javascript // 각기 다른 서버에 연결 const ws1 = new WebSocket('ws://localhost:7910/ws?token=JWT_TOKEN'); ...ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (1)
50-72: Broadcast 미구현(TODO) — 최소 구현 또는 명시적 예외 권장현재 no-op으로 로그만 남습니다. 사용 경로에서 오해 소지 있습니다. 최소한 NotSupportedException 또는 주석으로 명확히 하거나 IConnectionRegistry 연동을 빠르게 추가하세요.
- _logger.LogDebug("로컬 브로드캐스트 메시지 (현재 구현 제한)"); + _logger.LogDebug("로컬 브로드캐스트 메시지 (현재 구현 제한, no-op)");또는:
throw new NotSupportedException("Local broker does not support broadcast yet.");ProjectVG.Domain/Models/Server/ServerInfo.cs (1)
3-40: 중복 모델 정의 가능성: Application 계층의 ServerInfo와 불일치 위험Domain과 Application에 동명 모델이 공존하면 직렬화/매핑 혼선이 생길 수 있습니다. 하나로 통합하거나 공유 계약(공용 프로젝트)로 이동을 권장합니다. 상태(Status) 문자열은 enum 고려.
두 클래스 필드/의미가 1:1 일치하는지 확인해 주세요.
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (1)
164-170: 하트비트 업데이트 호출은 적절함 — 비차단(타임아웃) + 로컬 세션 타임아웃 갱신 제안Redis/브로커 지연 시 세션 루프가 불필요하게 막힐 수 있습니다. 짧은 타임아웃으로 대기하고, 핑 수신마다 로컬 CancelAfter를 갱신해 유휴 타임아웃을 “활동 기준”으로 유지하는 것을 권장합니다.
아래처럼 변경을 제안합니다(2초 타임아웃 + 핑 수신 시 30분 연장):
- // 세션 하트비트 업데이트 (Redis TTL 갱신) - try { - await _webSocketService.UpdateSessionHeartbeatAsync(userId); - } - catch (Exception heartbeatEx) { - _logger.LogWarning(heartbeatEx, "세션 하트비트 업데이트 실패: {UserId}", userId); - } + // 세션 하트비트 업데이트 (Redis TTL 갱신) + try { + var heartbeatTask = _webSocketService.UpdateSessionHeartbeatAsync(userId); + var completed = await Task.WhenAny( + heartbeatTask, + Task.Delay(TimeSpan.FromSeconds(2), cancellationTokenSource.Token)); + if (completed != heartbeatTask) { + _logger.LogWarning("세션 하트비트 업데이트 지연: {UserId}", userId); + } else { + await heartbeatTask; // 예외 관찰 + } + } + catch (Exception heartbeatEx) { + _logger.LogWarning(heartbeatEx, "세션 하트비트 업데이트 실패: {UserId}", userId); + } + finally { + // 핑 수신 시 세션 유휴 타임아웃 갱신(활동 기반) + cancellationTokenSource.CancelAfter(TimeSpan.FromMinutes(30)); + }ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs (1)
215-236: 중복/주석 처리된 분산 등록 메서드 블록 — 주석 제거 또는 소스 이동 명시코멘트 블록이 파일 내 두 군데에 중복되어 가독성을 해칩니다. 실제 사용 위치로 이관되었다면 제거를 권장합니다.
불필요한 블록 제거 제안:
- /* - /// <summary> - /// 분산 시스템 서비스 - /// </summary> - private static void AddDistributedSystemServices(IServiceCollection services, IConfiguration configuration) - { - var distributedEnabled = configuration.GetValue<bool>("DistributedSystem:Enabled", false); - - if (distributedEnabled) - { - // 분산 시스템이 활성화된 경우에만 등록 - services.AddScoped<ProjectVG.Domain.Services.Server.IServerRegistrationService, ProjectVG.Infrastructure.Services.Server.RedisServerRegistrationService>(); - services.AddHostedService<ProjectVG.Infrastructure.Services.Server.ServerLifecycleService>(); - - Console.WriteLine("분산 시스템 모드 활성화"); - } - else - { - Console.WriteLine("단일 서버 모드"); - } - } - */ProjectVG.Application/Models/Server/ServerInfo.cs (3)
13-15: 기본 생성자에서 타임스탬프 초기화 누락StartedAt/LastHeartbeat를 기본값(DateTime.MinValue)로 두면 직후 상태 판단이 불일치할 수 있습니다. 기본 생성자에서도 UTC now로 초기화하세요.
적용 diff:
public ServerInfo() { -} + StartedAt = DateTime.UtcNow; + LastHeartbeat = DateTime.UtcNow; +}
31-34: 음수 연결 수 방지음수 입력 방어로직을 추가해 주세요.
적용 diff:
public void UpdateConnectionCount(int count) { - ActiveConnections = count; + ActiveConnections = Math.Max(0, count); }
1-41: 도메인 모델(ServerInfo) 중복 정의 최소화 제안Application/Domain에 동일 개념의 ServerInfo가 공존하면 유지보수 시 동기화 리스크가 큽니다. 가능하면 Domain 모델을 단일 소스로 사용하거나, 명확한 DTO로 분리/매핑 계층을 도입해 주세요.
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (1)
47-49: 중복 서비스 등록 제거(ICharacterService/IUserService)위에서 이미 등록된 동일한 서비스가 다시 등록되고 있습니다. 혼란 방지를 위해 제거하세요.
적용 diff:
- services.AddScoped<ICharacterService, CharacterService>(); - services.AddScoped<IUserService, UserService>();docs/architecture/chat-system-flow.md (3)
222-225: 코드 예시가 현재 구현과 불일치 (브로커 사용)ChatSuccessHandler는 IMessageBroker를 사용합니다. 문서 예시를 브로커 호출로 교체하세요.
적용 diff:
-// ChatSuccessHandler에서 분산 메시지 전송 -await _distributedWebSocketManager.SendToUserAsync(userId, chatResult); +// ChatSuccessHandler에서 브로커 기반 전송 +await _messageBroker.SendToUserAsync(userId, wsMessage);
193-195: 응답 코드 설명 일치화예시는 Ok(result)로 200을 반환합니다. 문서의 “즉시 응답 (202)” 표기를 현실과 일치시키세요.
적용 diff:
- │ 2. 즉시 응답 (202) │ │ + │ 2. 즉시 응답 (200) │ │
36-38: 샘플 코드 변수명 오타userGuid → userId로 정정하세요.
적용 diff:
- var userId = User.FindFirst(ClaimTypes.NameIdentifier)?.Value; - var command = new ChatRequestCommand(userGuid, request.CharacterId, request.Message, request.RequestAt, request.UseTTS); + var userId = User.FindFirst(ClaimTypes.NameIdentifier)?.Value; + var command = new ChatRequestCommand(userId, request.CharacterId, request.Message, request.RequestAt, request.UseTTS);ProjectVG.Infrastructure/Services/Server/ServerLifecycleService.cs (1)
31-41: 서버 등록 실패 시 시작 단계 예외 처리 보강StartAsync에서 RegisterServerAsync 예외가 전파되면 서비스 시작 자체가 실패합니다. 로깅 후 재시도 또는 실패 허용 전략을 추가하세요.
적용 diff:
- using var scope = _scopeFactory.CreateScope(); - var serverRegistration = scope.ServiceProvider.GetRequiredService<IServerRegistrationService>(); - await serverRegistration.RegisterServerAsync(); + try + { + using var scope = _scopeFactory.CreateScope(); + var serverRegistration = scope.ServiceProvider.GetRequiredService<IServerRegistrationService>(); + await serverRegistration.RegisterServerAsync(); + } + catch (Exception ex) + { + _logger.LogError(ex, "서버 등록 중 오류"); + // 필요 시 재시도/지속 실행 전략 결정 + }ProjectVG.Application/Services/Server/IServerRegistrationService.cs (1)
1-52: 도메인 인터페이스와 이중 정의 — 통합 권장동일 명세의 인터페이스가 Application/Domain에 중복되어 있습니다. 한 층(보통 Domain)으로 수렴하고, 다른 층에서는 재노출하지 않거나 어댑터로 감싸는 방식이 단순합니다. 모델(ServerInfo)도 단일 소스로 통합하세요.
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (8)
49-54: Redis 채널 문자열 암묵 변환 사용 중단 (obsolete 경고 해결)RedisChannel.Literal로 명시하세요.
적용 diff:
- var serverChannel = $"{SERVER_CHANNEL_PREFIX}:{_serverId}"; - _subscriber.Subscribe(serverChannel, OnServerMessageReceived); + var serverChannel = RedisChannel.Literal($"{SERVER_CHANNEL_PREFIX}:{_serverId}"); + _subscriber.Subscribe(serverChannel, OnServerMessageReceived); - // 브로드캐스트 메시지 구독 - _subscriber.Subscribe(BROADCAST_CHANNEL, OnBroadcastMessageReceived); + // 브로드캐스트 메시지 구독 + var broadcastChannel = RedisChannel.Literal(BROADCAST_CHANNEL); + _subscriber.Subscribe(broadcastChannel, OnBroadcastMessageReceived);
94-103: Publish 채널도 Literal로 명시문자열 암묵 변환 대신 Literal 사용.
적용 diff:
- var brokerMessage = BrokerMessage.CreateUserMessage(userId, message, _serverId); - var userChannel = $"{USER_CHANNEL_PREFIX}:{userId}"; + var brokerMessage = BrokerMessage.CreateUserMessage(userId, message, _serverId); + var userChannel = RedisChannel.Literal($"{USER_CHANNEL_PREFIX}:{userId}"); _logger.LogInformation("[분산브로커] Redis Pub 시작: Channel={Channel}, TargetServerId={TargetServerId}, SourceServerId={SourceServerId}", userChannel, targetServerId, _serverId); await _subscriber.PublishAsync(userChannel, brokerMessage.ToJson());
116-119: 브로드캐스트 Publish도 Literal 사용적용 diff:
- var brokerMessage = BrokerMessage.CreateBroadcastMessage(message, _serverId); - await _subscriber.PublishAsync(BROADCAST_CHANNEL, brokerMessage.ToJson()); + var brokerMessage = BrokerMessage.CreateBroadcastMessage(message, _serverId); + var broadcastChannel = RedisChannel.Literal(BROADCAST_CHANNEL); + await _subscriber.PublishAsync(broadcastChannel, brokerMessage.ToJson());
132-137: 서버 대상 Publish도 Literal 사용적용 diff:
- var brokerMessage = BrokerMessage.CreateServerMessage(serverId, message, _serverId); - var serverChannel = $"{SERVER_CHANNEL_PREFIX}:{serverId}"; + var brokerMessage = BrokerMessage.CreateServerMessage(serverId, message, _serverId); + var serverChannel = RedisChannel.Literal($"{SERVER_CHANNEL_PREFIX}:{serverId}"); await _subscriber.PublishAsync(serverChannel, brokerMessage.ToJson());
152-159: 사용자 채널 Subscribe도 Literal 사용적용 diff:
- var userChannel = $"{USER_CHANNEL_PREFIX}:{userId}"; - await _subscriber.SubscribeAsync(userChannel, OnUserMessageReceived); + var userChannel = RedisChannel.Literal($"{USER_CHANNEL_PREFIX}:{userId}"); + await _subscriber.SubscribeAsync(userChannel, OnUserMessageReceived);
173-175: 사용자 채널 Unsubscribe도 Literal 사용적용 diff:
- var userChannel = $"{USER_CHANNEL_PREFIX}:{userId}"; - await _subscriber.UnsubscribeAsync(userChannel); + var userChannel = RedisChannel.Literal($"{USER_CHANNEL_PREFIX}:{userId}"); + await _subscriber.UnsubscribeAsync(userChannel);
227-247: async/await 없는 async void 제거 (정적 분석 경고 해결)대기 동작이 없으므로 async 제거가 적절합니다.
적용 diff:
- private async void OnServerMessageReceived(RedisChannel channel, RedisValue message) + private void OnServerMessageReceived(RedisChannel channel, RedisValue message) { try { var brokerMessage = BrokerMessage.FromJson(message!);
249-268: async/await 없는 async void 제거 (정적 분석 경고 해결)대기 동작이 없으므로 async 제거가 적절합니다.
적용 diff:
- private async void OnBroadcastMessageReceived(RedisChannel channel, RedisValue message) + private void OnBroadcastMessageReceived(RedisChannel channel, RedisValue message) { try { var brokerMessage = BrokerMessage.FromJson(message!);ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs (1)
105-123: 테스트 명칭 갱신 제안: WebSocket → Broker.IMessageBroker 경로로 전환됨에 따라 테스트명 ‘HandleAsync_WithWebSocketFailure…’는 혼동 소지가 있습니다. ‘…WithBrokerFailure…’로 변경 권장.
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (2)
48-76: Redis KEYS 스캔 사용은 규모에 취약 (SCAN/집합 기반으로 전환 권장).GetAllAsync에서 server.Keys(pattern) 사용은 대규모/Cluster에서 비용 큼. 세션 키 집합(예: sessions:active Set)에 등록/해제하는 방식으로 조회/카운트 수행 권장.
159-169: 세션 카운트도 KEYS 스캔 대체 권장.GetActiveSessionCountAsync도 동일하게 Set 크기 기반(count)으로 교체하면 비용 절감 가능.
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (2)
122-131: 구현타입 캐스팅 제거 제안 (IMessageBroker로 일관).messageBroker as DistributedMessageBroker 캐스팅은 결합도를 높입니다. IMessageBroker에 사용자 채널 구독/해제 기능(기본 no-op)을 추가하거나 별도 ISubscriberBroker 인터페이스로 분리해 주입받는 쪽은 인터페이스만 참조하도록 리팩터링 권장.
116-138: Heartbeat 구현을 인터페이스에 승격.Redis 여부 분기 및 구체타입 체크 대신 ISessionStorage에 HeartbeatAsync(userId) 추가(또는 TryHeartbeatAsync)하여 다형성으로 처리하세요.
ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (2)
263-276: user:server: 전수 스캔은 비용 큼 — 매핑 인덱스 구조를 도입하세요.*서버별 사용자 집합(e.g., server:{serverId}:users)을 유지해 정리 시 해당 집합만 순회하면 됩니다. 매핑 설정/해제 시 이 집합을 함께 업데이트하세요.
229-256: 오프라인 서버 정리 로직 보완 아이디어.ACTIVE_SERVERS_SET와 서버 키 존재만으로 판단 중입니다. ServerInfo TTL 연장 실패 대비해 LastHeartbeat 기반 타임아웃(IsHealthy) 검증 추가 고려.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (34)
.env.example(1 hunks).gitignore(1 hunks)ProjectVG.Api/Middleware/WebSocketMiddleware.cs(1 hunks)ProjectVG.Api/Program.cs(1 hunks)ProjectVG.Api/appsettings.json(1 hunks)ProjectVG.Application/ApplicationServiceCollectionExtensions.cs(3 hunks)ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs(1 hunks)ProjectVG.Application/Models/Server/ServerInfo.cs(1 hunks)ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs(2 hunks)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs(1 hunks)ProjectVG.Application/Services/MessageBroker/IMessageBroker.cs(1 hunks)ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs(1 hunks)ProjectVG.Application/Services/Server/IServerRegistrationService.cs(1 hunks)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs(1 hunks)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs(1 hunks)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs(1 hunks)ProjectVG.Common/Models/Session/SessionInfo.cs(1 hunks)ProjectVG.Domain/Models/Server/ServerInfo.cs(1 hunks)ProjectVG.Domain/Services/Server/IServerRegistrationService.cs(1 hunks)ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs(2 hunks)ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs(1 hunks)ProjectVG.Infrastructure/ProjectVG.Infrastructure.csproj(1 hunks)ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs(1 hunks)ProjectVG.Infrastructure/Services/Server/ServerLifecycleService.cs(1 hunks)ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs(10 hunks)docker-compose.db.yml(1 hunks)docker-compose.yml(3 hunks)docs/architecture/chat-system-flow.md(1 hunks)docs/distributed-system/README.md(1 hunks)scripts/monitor-redis.ps1(1 hunks)scripts/start-server-2.ps1(1 hunks)scripts/start-server-3.ps1(1 hunks)scripts/test-distributed-system.ps1(1 hunks)test-clients/ai-chat-client/script.js(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (12)
ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (2)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (3)
Task(31-53)Task(94-104)Task(109-205)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (4)
Task(10-10)Task(15-15)Task(20-20)Task(30-30)
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (7)
ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs (1)
IServiceCollection(25-36)ProjectVG.Tests/Api/Configuration/ConfigurationExtensionsTests.cs (1)
IConfiguration(435-440)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (2)
DistributedMessageBroker(13-316)DistributedMessageBroker(28-42)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (2)
DistributedWebSocketManager(14-139)DistributedWebSocketManager(21-33)ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (2)
LocalMessageBroker(10-80)LocalMessageBroker(17-23)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (2)
WebSocketManager(9-85)WebSocketManager(15-23)ProjectVG.Application/Services/Session/ConnectionRegistry.cs (2)
ConnectionRegistry(6-65)ConnectionRegistry(11-14)
ProjectVG.Infrastructure/Services/Server/ServerLifecycleService.cs (2)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (6)
Task(66-110)Task(112-126)Task(128-143)Task(148-164)Task(169-185)Task(270-301)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (6)
Task(35-59)Task(61-66)Task(68-79)Task(81-92)Task(94-109)Task(116-138)
ProjectVG.Application/Services/Server/IServerRegistrationService.cs (1)
ProjectVG.Application/Models/Server/ServerInfo.cs (3)
ServerInfo(3-40)ServerInfo(13-15)ServerInfo(17-24)
ProjectVG.Domain/Services/Server/IServerRegistrationService.cs (2)
ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (1)
GetServerId(125-128)ProjectVG.Domain/Models/Server/ServerInfo.cs (3)
ServerInfo(3-40)ServerInfo(13-15)ServerInfo(17-24)
ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (2)
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (1)
IsSessionActive(111-114)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (1)
IsSessionActive(74-77)
ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (3)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (6)
Task(66-110)Task(112-126)Task(128-143)Task(148-164)Task(169-185)Task(270-301)ProjectVG.Domain/Models/Server/ServerInfo.cs (3)
ServerInfo(3-40)ServerInfo(13-15)ServerInfo(17-24)ProjectVG.Domain/Services/Server/IServerRegistrationService.cs (1)
GetServerId(25-25)
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs (2)
ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (2)
ChatSuccessHandler(10-129)ChatSuccessHandler(16-24)ProjectVG.Application/Models/WebSocket/WebSocketMessage.cs (2)
WebSocketMessage(16-16)WebSocketMessage(18-23)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (7)
ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (1)
GetServerId(125-128)ProjectVG.Application/Services/Server/IServerRegistrationService.cs (3)
GetServerId(25-25)Task(10-10)Task(15-15)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (7)
Task(35-59)Task(61-66)Task(68-79)Task(81-92)Task(94-109)Task(116-138)IsSessionActive(111-114)ProjectVG.Application/Services/MessageBroker/IMessageBroker.cs (3)
Task(8-8)Task(13-13)Task(18-18)ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (3)
Task(25-48)Task(50-72)Task(74-79)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (1)
IsSessionActive(25-25)ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs (6)
BrokerMessage(5-88)BrokerMessage(16-29)BrokerMessage(31-44)BrokerMessage(46-58)BrokerMessage(77-87)ToJson(72-75)
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (3)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (3)
Task(31-53)Task(94-104)Task(109-205)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (6)
Task(35-59)Task(61-66)Task(68-79)Task(81-92)Task(94-109)Task(116-138)ProjectVG.Common/Models/Session/SessionInfo.cs (1)
SessionInfo(3-9)
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (3)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (8)
DistributedMessageBroker(13-316)DistributedMessageBroker(28-42)Task(66-110)Task(112-126)Task(128-143)Task(148-164)Task(169-185)Task(270-301)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (1)
IsSessionActive(25-25)ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (2)
RedisSessionStorage(8-222)RedisSessionStorage(17-22)
ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (4)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (6)
Task(66-110)Task(112-126)Task(128-143)Task(148-164)Task(169-185)Task(270-301)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (6)
Task(35-59)Task(61-66)Task(68-79)Task(81-92)Task(94-109)Task(116-138)ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (2)
Task(26-90)Task(95-128)ProjectVG.Application/Services/MessageBroker/IMessageBroker.cs (2)
Task(8-8)Task(13-13)
🪛 GitHub Actions: Develop
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs
[error] 209-209: Moq ArgumentException: Invalid callback. Setup on method with parameters (string, object) cannot invoke callback with parameters (string, WebSocketMessage).
[error] 138-138: Moq ArgumentException: Invalid callback. Setup on method with parameters (string, object) cannot invoke callback with parameters (string, WebSocketMessage).
[error] 91-91: Moq ArgumentException: Invalid callback. Setup on method with parameters (string, object) cannot invoke callback with parameters (string, WebSocketMessage).
[error] 163-163: Moq ArgumentException: Invalid callback. Setup on method with parameters (string, object) cannot invoke callback with parameters (string, WebSocketMessage).
[error] 186-186: Moq ArgumentException: Invalid callback. Setup on method with parameters (string, object) cannot invoke callback with parameters (string, WebSocketMessage).
🪛 GitHub Check: Build & Test
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs
[warning] 249-249:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
[warning] 227-227:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
[warning] 174-174:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 153-153:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 135-135:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 117-117:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 101-101:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 53-53:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 50-50:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs
[warning] 159-159:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
🪛 markdownlint-cli2 (0.18.1)
docs/distributed-system/README.md
11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
61-61: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
125-125: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
184-184: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 7-7: [UnorderedKey] The DEBUG_MODE key should go before the ENVIRONMENT key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The LOG_FORMAT key should go before the LOG_LEVEL key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The DATA_PATH key should go before the DEBUG_MODE key
(UnorderedKey)
[warning] 13-13: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🔇 Additional comments (23)
.gitignore (1)
104-104: LGTM:.dockerignore.local무시 항목 추가 적절개발자별 로컬 Docker 제외 규칙을 VCS에서 분리하는 목적에 부합합니다.
ProjectVG.Infrastructure/ProjectVG.Infrastructure.csproj (1)
17-17: LGTM: Hosting.Abstractions 의존성 추가IHostedService/BackgroundService 구현에 필요한 최소 참조로 적절합니다. 동일 라인의 패키지들과 버전 정합성도 맞습니다.
docker-compose.db.yml (1)
3-6: LGTM: 최상위 volumes 선언으로 구성 단순화mssql_data/redis_data를 상단에서 선언해 가독성과 재사용성이 좋아졌습니다.
scripts/test-distributed-system.ps1 (1)
41-43: 보조 스크립트 존재 여부 확인 요청가이드에 언급된
.\scripts\monitor-redis.ps1가 레포지토리에 있는지 확인 부탁드립니다. 누락 시 테스트 가이드가 혼란을 줄 수 있습니다.test-clients/ai-chat-client/script.js (4)
338-343: 수신 로그 개선 LGTM타임스탬프+이모지 표기 일관성 좋아졌습니다.
557-561: 송신 로그 개선 LGTM송신 시점/메시지 표시가 명확합니다.
570-571: 페이로드 디버그 로그 LGTM문제 분석에 유용합니다. 운영 전환 시 레벨/마스킹 고려만 부탁드립니다.
594-595: 에러 로그 개선 LGTM타임스탬프 포함 및 시멘틱 이모지로 가독성이 좋습니다.
docker-compose.yml (3)
31-36: 추가 환경변수 도입 LGTMENVIRONMENT/DEBUG_MODE/LOG_LEVEL/LOG_FORMAT 도입은 운영 가시성에 유용합니다. 애플리케이션에서 실제로 읽어 사용하는지 최종 확인 부탁드립니다.
41-41: STT_BASE_URL 추가 반영 LGTM외부 의존성 일관성 확보에 좋습니다.
56-57: 로그 볼륨 마운트 LGTM컨테이너 재시작 시 로그 보존에 유용합니다.
ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (2)
25-48: 로컬 사용자 전송 처리 LGTMWebSocketMessage 여부 판단 후 래핑 전송 구조 적절합니다. 예외 로깅/전파도 좋습니다.
74-79: 서버 간 통신 무시 로그 LGTM로컬 모드에서 명시적 무시 처리 적절합니다.
해당 메서드가 퍼블릭 API에 노출된다면 사용 측에서 혼동하지 않도록 XML 주석에 명확히 기재 부탁드립니다.
ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (1)
27-31: 세션 하트비트 API 추가 LGTM분산 TTL 갱신 목적에 적합합니다. 구현부(분산/로컬)에서 안전하게 no-op/구현되어 있는지 최종 확인만 부탁드립니다.
ProjectVG.Api/appsettings.json (1)
14-20: DistributedSystem 설정 추가 LGTM기본값/간격 합리적입니다. ServerId는 배포마다 고유하게 주입되는지 확인해 주세요.
ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (1)
79-84: 로컬 매니저 no-op 하트비트 추가 — 적절합니다분산/로컬 공통 인터페이스 정합성을 맞추는 깔끔한 선택입니다. Debug 레벨이므로 로그 소음도 우려 없습니다.
ProjectVG.Api/Program.cs (1)
41-41: IConfiguration 주입 및 분산 배선 확인 — LGTMAddApplicationServices 시그니처가 ProjectVG.Application/ApplicationServiceCollectionExtensions.cs에 있는
public static IServiceCollection AddApplicationServices(this IServiceCollection services, IConfiguration configuration)
로 업데이트되어 있으며, 내부에서 AddDistributedServices(services, configuration)를 호출해 DistributedSystem:Enabled에 따라 IMessageBroker/DistributedWebSocketManager(분산) 또는 LocalMessageBroker/WebSocketManager(단일)를 등록합니다.ProjectVG.Common/Models/Session/SessionInfo.cs (1)
3-9: record → class 전환 영향 검증 — 결과
- 검색 결과: 'with { ... }' 사용은 ProjectVG.Application/Models/Chat/ChatProcessResultMessage.cs(라인 69, 73, 109)에서 발견되었으나 대상은 ChatProcessResultMessage이며 SessionInfo에 대한 'with' 사용 또는 SessionInfo를 키/동등성 비교(Equals/Dictionary/HashSet/IEqualityComparer)로 사용한 흔적은 검색되지 않았습니다.
- 결론: 이 리포지토리 내에서는 SessionInfo를 record에서 class(+set 허용)로 전환해도 즉각적인 with/동등성 관련 브레이킹은 없습니다.
- 권장(선택): DateTime → DateTimeOffset 검토; 외부(다른 레포/직렬화 소비자)가 있다면 호환성 확인; 클래스 확장 금지 의도면 sealed 지정.
ProjectVG.Domain/Services/Server/IServerRegistrationService.cs (1)
1-52: 명세 적절서버 수명주기/매핑에 필요한 계약이 명확합니다.
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (1)
28-43: 구독 초기화가 생성자에서 수행됨 — DI 라이프사이클과 정합성 확인현재 Scoped 등록 시 요청마다 구독이 중복됩니다. ApplicationServiceCollectionExtensions에서 Singleton으로 전환이 필요합니다(별도 코멘트 참고).
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (1)
23-24: 확장 메서드 시그니처 변경 — 호출부 확인됨
ProjectVG.Api/Program.cs(라인 41)에 builder.Services.AddApplicationServices(builder.Configuration); 호출이 있어 변경이 반영되어 있습니다.ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (1)
66-73: IMessageBroker.IsDistributed가 존재하므로 BrokerType 로그 유지 가능
IMessageBroker에 bool IsDistributed { get; }가 정의되어 있고 DistributedMessageBroker / LocalMessageBroker가 구현하고 있으므로 컴파일 오류가 없습니다.ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs (1)
1-89: 전반적으로 적절합니다.단순·명확한 모델과 안전한 JSON 파싱(default/null 반환) 좋아요.
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
| using ProjectVG.Application.Services.MessageBroker; | ||
| using ProjectVG.Application.Services.Session; | ||
| using ProjectVG.Common.Models.Session; | ||
| using ProjectVG.Infrastructure.Persistence.Session; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
레이어링 위반: Application에서 Infrastructure 네임스페이스 직접 참조.
Application 계층이 ProjectVG.Infrastructure.*를 참조하면 의존성 방향이 뒤집힙니다. Heartbeat 기능을 ISessionStorage(또는 IHeartbeatSessionStorage)로 승격하여 인프라 세부 구현 의존 제거하세요.
🤖 Prompt for AI Agents
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs around
line 6: the file currently references
ProjectVG.Infrastructure.Persistence.Session which is a layering violation;
replace the concrete Infrastructure dependency by depending on an interface
(e.g., ISessionStorage or IHeartbeatSessionStorage) defined in the Application
(or Application.Interfaces) project, move the heartbeat-related
members/signature into that interface, update DistributedWebSocketManager to
inject and call the interface methods instead of the concrete class, remove the
Infrastructure using, and register the concrete implementation in the
composition root (Startup/DI) within the Infrastructure project so Application
no longer references Infrastructure.
ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs
Outdated
Show resolved
Hide resolved
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs
Outdated
Show resolved
Hide resolved
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs
Outdated
Show resolved
Hide resolved
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs
Outdated
Show resolved
Hide resolved
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
ProjectVG.Tests/Api/Services/TestClientLauncherTests.cs (3)
64-76: 시간 단언 임계값(50ms) 과도 — CI 플레이키 가능성, TimeSpan 기반으로 완화 권장JIT/스케줄링 노이즈로 50ms를 초과할 수 있습니다. TimeSpan 비교로 바꾸고 임계값을 여유 있게 잡는 편이 안전합니다.
- stopwatch.ElapsedMilliseconds.Should().BeLessOrEqualTo(50, - "Launch 메서드 자체는 즉시 반환되어야 함 (백그라운드 작업은 별도)"); + stopwatch.Elapsed.Should().BeLessThan(TimeSpan.FromMilliseconds(200), + "Launch 메서드는 즉시 반환되어야 하지만 CI 노이즈를 고려해 여유 임계값 사용");
77-79: 100ms 대기는 목적에 부합하지 않음백그라운드 작업은 Task.Delay(1000) 이후 시작되므로 100ms 대기는 의미가 없습니다. 불필요한 테스트 지연을 제거하세요.
- // Give some time for the background task to start - await Task.Delay(100); + // 불필요한 대기 제거 (백그라운드는 1초 후 시작)
72-74: 원인 가능성: Launch의 ContinueWith 스케줄러 지정 없음현재 구현(ProjectVG.Api/Services/TestClientLauncher.cs)은 Task.Delay(...).ContinueWith(...)를 기본 스케줄러(TaskScheduler.Current)에 올립니다. xUnit의 SynchronizationContext 영향으로 드물게 연기/누락될 수 있어 플레이키 요인입니다. TaskScheduler.Default로 고정하거나 Task.Run을 사용해 테스트 컨텍스트와 분리하는 것을 권장합니다.
예시(서비스 코드 측 변경 참고):
public void Launch() { _ = Task.Run(async () => { await Task.Delay(1000).ConfigureAwait(false); try { var htmlPath = Path.Combine(Directory.GetCurrentDirectory(), "..", "test-clients", "test-client.html"); if (File.Exists(htmlPath)) { System.Diagnostics.Process.Start(new System.Diagnostics.ProcessStartInfo { FileName = htmlPath, UseShellExecute = true, WindowStyle = System.Diagnostics.ProcessWindowStyle.Normal }); } } catch { } }); }또는:
_ = Task.Delay(1000).ContinueWith(_ => { /* ... */ }, TaskScheduler.Default);ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs (2)
46-47: Base64 패딩 영향 최소화를 위해 입력 크기를 3의 배수로 보정 제안패딩으로 인한 변동을 줄이기 위해 입력을 3의 배수로 맞추면 비교가 더 안정적입니다. 대용량으로 늘린 만큼 CI 런타임도 증가할 수 있으니 필요 시 반복 횟수(TestIterations) 조정이나 성능 테스트 태깅/분리도 고려해 주세요.
적용 예:
- // 더 큰 데이터 크기로 ArrayPool의 이점을 확인 - var largeTestData = GenerateTestAudioData(AudioDataSize * 4); // 512KB로 확대 + // 더 큰 데이터 크기로 ArrayPool의 이점을 확인 + // Base64 패딩 영향 최소화를 위해 3의 배수로 보정 + var baseSize = AudioDataSize * 4; // 512KB 목표 + if (baseSize % 3 != 0) baseSize += 3 - (baseSize % 3); + var largeTestData = GenerateTestAudioData(baseSize);
60-67: 성능 허용 기준 계산을 분모 0 안전/간결하게 개선현재 % 계산은 분모가 극단적으로 작을 때 불안정할 수 있습니다. ratio(비율) 기반으로 단순화하고 분모 0을 방지하는 편이 안전합니다. 주석도 실제 허용 조건과 일치하도록 정리했습니다.
- // 큰 크기 데이터에서는 ArrayPool의 이점이 더 명확해짐 - // 성능 차이가 50% 이상 나거나 ArrayPool이 더 빠르면 통과 - var performanceImprovement = ((convertTime.TotalMilliseconds - pooledBase64Time.TotalMilliseconds) / convertTime.TotalMilliseconds * 100); - - Assert.True(pooledBase64Time <= convertTime || performanceImprovement >= -50.0, - $"ArrayPool Base64 방식({pooledBase64Time.TotalMilliseconds:F2}ms)이 " + - $"Convert 방식({convertTime.TotalMilliseconds:F2}ms)보다 50% 이상 느립니다. " + - $"성능 차이: {performanceImprovement:F1}%"); + // 큰 데이터에서의 경향 확인: + // ArrayPool이 더 빠르거나(Convert 대비 ratio ≤ 1.0), + // Convert 대비 최대 50%까지 느린 경우(ratio ≤ 1.5) 허용 + // 분모가 0인 극단값을 피하기 위해 Ticks 기반 비율 사용 + var ratio = (double)pooledBase64Time.Ticks / Math.Max(convertTime.Ticks, 1L); // 허용: ≤ 1.5 + var performanceImprovement = (1.0 - ratio) * 100.0; + + Assert.True(pooledBase64Time <= convertTime || ratio <= 1.5, + $"ArrayPool Base64 방식({pooledBase64Time.TotalMilliseconds:F2}ms)이 " + + $"Convert 방식({convertTime.TotalMilliseconds:F2}ms) 대비 허용 비율(≤ 150%)을 초과했습니다. " + + $"ratio={ratio:F2}, Δ%={performanceImprovement:F1}%");
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
ProjectVG.Tests/Api/Services/TestClientLauncherTests.cs(1 hunks)ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs(1 hunks)ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs(10 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ProjectVG.Tests/Api/Services/TestClientLauncherTests.cs (1)
ProjectVG.Api/Services/TestClientLauncher.cs (1)
Launch(5-23)
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs (2)
ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (2)
ChatSuccessHandler(10-129)ChatSuccessHandler(16-24)ProjectVG.Application/Models/WebSocket/WebSocketMessage.cs (2)
WebSocketMessage(16-16)WebSocketMessage(18-23)
🔇 Additional comments (15)
ProjectVG.Tests/Infrastructure/Integrations/MemoryPoolingPerformanceTests.cs (3)
50-50: LGTM — 큰 입력 기준으로 Convert 경로 측정변경 타당합니다. 동일 데이터 기준 비교가 보장됩니다.
53-53: LGTM — Pooled 경로도 동일 입력으로 측정변경 타당합니다. 비교의 공정성이 유지됩니다.
69-69: LGTM — 출력에 데이터 크기 포함가독성 좋습니다. 필요 시 MB 단위 병행 표기도 고려해 볼 수 있습니다.
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs (12)
9-9: LGTM!IMessageBroker 서비스 추가가 적절합니다.
17-17: LGTM!IWebSocketManager에서 IMessageBroker로의 의존성 변경이 적절하게 반영되었습니다.
Also applies to: 24-24, 26-26
38-38: LGTM!메서드 시그니처가 IMessageBroker의 SendToUserAsync에 맞게 수정되었습니다.
55-57: LGTM!SendToUserAsync 메서드 검증이 올바르게 수정되었습니다.
76-78: LGTM!유효한 세그먼트에 대한 검증이 적절합니다.
91-92: Moq Callback 제네릭 인자 타입 불일치로 테스트 실패 가능성.IMessageBroker.SendToUserAsync의 두 번째 매개변수는 object입니다. 기존 리뷰에서 지적된 동일한 문제가 여전히 존재합니다.
111-111: LGTM!예외 시나리오 테스트가 적절하게 수정되었습니다.
118-120: LGTM!예외 발생 후 검증 로직이 올바릅니다.
138-143: Moq Callback 제네릭 인자 타입 불일치.기존 리뷰 코멘트와 동일한 문제가 여전히 존재합니다. IMessageBroker.SendToUserAsync의 두 번째 매개변수가 object이므로 타입 캐스팅이 필요합니다.
167-172: Moq Callback 제네릭 인자 타입 불일치.기존 리뷰 코멘트와 동일한 문제입니다. IMessageBroker 인터페이스의 메서드 시그니처에 맞게 수정이 필요합니다.
194-195: Moq Callback 제네릭 인자 타입 불일치.기존 리뷰 코멘트에서 지적된 동일한 문제가 여전히 남아있습니다.
217-222: Moq Callback 제네릭 인자 타입 불일치.기존 리뷰에서 지적된 동일한 문제입니다. IMessageBroker의 메서드 시그니처에 맞는 타입 수정이 필요합니다.
MessageBroker와 WebSocketManager를 Scoped에서 Singleton으로 변경 - DistributedMessageBroker의 Redis 구독 중복 방지 - 분산 환경에서 메시지 중복 처리 방지 - 프로세스 전역 상태 일관성 보장 - 자원 누수 및 성능 문제 해결 변경된 서비스: - IMessageBroker (DistributedMessageBroker, LocalMessageBroker) - IWebSocketManager (DistributedWebSocketManager, WebSocketManager)
GetActiveSessionCountAsync 메서드에서 불필요한 async 키워드 제거 - async 키워드 제거 및 Task.FromResult() 사용 - 실제 비동기 작업이 없는 메서드의 올바른 패턴 적용 - CS1998 컴파일러 경고 해결
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (8)
87-89: 중복 생성 방지: StringSet에 When.NotExists 사용중복 세션 생성에 대비해 원자적으로 보호하세요.
- var json = JsonSerializer.Serialize(session); - await _database.StringSetAsync(key, json, SESSION_TTL); + var json = JsonSerializer.Serialize(session); + var created = await _database.StringSetAsync(key, json, expiry: SESSION_TTL, when: When.NotExists); + if (!created) + { + _logger.LogWarning("세션 생성 스킵(이미 존재): {SessionId}", session.SessionId); + }
107-111: 업데이트 안전성: 존재할 때만 갱신(When.Exists) 및 실패 로깅존재하지 않는 키를 생성하지 않도록 보장하세요.
- var json = JsonSerializer.Serialize(session); - await _database.StringSetAsync(key, json, SESSION_TTL); + var json = JsonSerializer.Serialize(session); + var updated = await _database.StringSetAsync(key, json, expiry: SESSION_TTL, when: When.Exists); + if (!updated) + { + _logger.LogWarning("세션 업데이트 실패(키 없음): {SessionId}", session.SessionId); + }
37-39: 할당 절감: UTF8 바이트에서 직접 역직렬화중간 문자열 생성 없이 역직렬화하면 GC 압박을 줄일 수 있습니다.
- var sessionInfo = JsonSerializer.Deserialize<SessionInfo>(value!); + var sessionInfo = JsonSerializer.Deserialize<SessionInfo>(value.Span);
177-186: userId == sessionId 전제에 의존하여 다중 세션을 반환하지 못함현재 구현은 userId로 단일 세션만 조회합니다. 다중 디바이스/세션을 허용하려면 사용자별 인덱스(Set) 유지가 필요합니다. 최소한 메서드명/주석을 단일 세션 전제로 정정하거나, 아래처럼 인덱스를 도입하세요.
예시(인덱스 사용 시):
- var session = await GetAsync(userId); - return session != null ? new[] { session } : Enumerable.Empty<SessionInfo>(); + var ids = await _database.SetMembersAsync($"session:index:user:{userId}"); + if (ids.Length == 0) return Enumerable.Empty<SessionInfo>(); + var keys = ids.Select(id => (RedisKey)(SESSION_KEY_PREFIX + (string)id)).ToArray(); + var values = await _database.StringGetAsync(keys); + return values + .Where(v => v.HasValue) + .Select(v => JsonSerializer.Deserialize<SessionInfo>(v.Span)) + .Where(s => s != null)!;
124-135: 삭제 시 인덱스 정합성 유지세션 삭제 성공 시, 보조 인덱스(전체/사용자별)에서 함께 제거해야 합니다. 전체 인덱스 제거는 적용하셨고(권장안 참조), 사용자 인덱스를 쓰는 경우 아래도 필요합니다.
예시:
if (deleted) { _logger.LogInformation("세션 삭제 성공: {SessionId}", sessionId); + // 사용자별 인덱스 유지 시: + var raw = await _database.StringGetAsync(key); + if (raw.HasValue) + { + var s = JsonSerializer.Deserialize<SessionInfo>(raw.Span); + if (!string.IsNullOrEmpty(s?.UserId)) + await _database.SetRemoveAsync($"session:index:user:{s!.UserId}", sessionId); + } }
52-53: 다중 DB 대비: Keys 호출에 database 파라미터 명시기본 DB가 0이 아닐 수 있으니 database: _database.Database를 지정하세요.
- var server = _redis.GetServer(_redis.GetEndPoints().First()); - var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*"); + var server = _redis.GetServer(_redis.GetEndPoints().First()); + var keys = server.Keys(database: _database.Database, pattern: SESSION_KEY_PREFIX + "*");동일하게 활성 세션 수 계산 지점도 반영하세요.
Also applies to: 163-165
14-16: 상수(프리픽스/TTL) 구성화
- TTL/프리픽스를 구성(IOptions)으로 노출하면 운영 중 조정이 용이합니다.
- 최소한 TTL은 설정값으로 관리 권장.
예시:
- options.Value.SessionTtlMinutes 사용 → TimeSpan.FromMinutes(options.Value.SessionTtlMinutes)
194-216: 하트비트 최적화(선택)하트비트에서 LastActivity 갱신이 불필요하다면 KeyExpireAsync만 호출해 직렬화/쓰기 비용을 줄일 수 있습니다. 현재 필드 갱신이 필요하다면 현 방식 유지.
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (2)
47-49: 중복 DI 등록(ICharacterService, IUserService) 제거동일 서비스가 앞에서 이미 AddScoped로 등록되어 있습니다. 중복은 유지보수 혼란을 유발하므로 제거 권장합니다.
- services.AddScoped<ICharacterService, CharacterService>(); - services.AddScoped<IUserService, UserService>();
86-86: 옵션 바인딩 사용 고려(선택)값 한 개를 직접 읽기보다는 DistributedSystem 섹션을 옵션으로 바인딩하면 테스트/유효성/재사용이 용이합니다. 예: services.Configure(configuration.GetSection("DistributedSystem")).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs(3 hunks)ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (6)
ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs (1)
IServiceCollection(25-36)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (2)
DistributedMessageBroker(13-316)DistributedMessageBroker(28-42)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (2)
DistributedWebSocketManager(14-139)DistributedWebSocketManager(21-33)ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (2)
LocalMessageBroker(10-80)LocalMessageBroker(17-23)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (2)
WebSocketManager(9-85)WebSocketManager(15-23)ProjectVG.Application/Services/Session/ConnectionRegistry.cs (2)
ConnectionRegistry(6-65)ConnectionRegistry(11-14)
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (3)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (3)
Task(31-53)Task(94-104)Task(109-205)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (3)
Task(25-36)Task(38-43)Task(45-54)ProjectVG.Common/Models/Session/SessionInfo.cs (1)
SessionInfo(3-9)
🔇 Additional comments (3)
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (1)
159-175: CS1998 경고 해결 확인 — 비동기 제거 OKasync 제거 및 Task.FromResult 반환으로 경고 해소된 것 확인했습니다. 👍
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (2)
91-99: 이전 리뷰 반영: Broker/WS Manager를 Singleton으로 등록한 점 좋습니다프로세스 전역 구독/상태를 공유하므로 싱글톤이 적절합니다. Redis 구독 중복/자원 누수 리스크가 줄어듭니다.
23-24: 확인 완료: 확장 메서드 시그니처 반영 및 분산 설정 존재Program.cs에서 AddApplicationServices(builder.Configuration)로 호출됨(ProjectVG.Api/Program.cs:41), AddApplicationServices 시그니처는 IConfiguration 파라미터로 정의되어 있음(ProjectVG.Application/ApplicationServiceCollectionExtensions.cs:23). ProjectVG.Api/appsettings.json에 "DistributedSystem" 항목(예: Enabled: false)이 존재하며, IServerRegistrationService는 분산 활성화 시 ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs:226에서 등록됩니다.
| services.AddSingleton<IMessageBroker, DistributedMessageBroker>(); | ||
| services.AddSingleton<IWebSocketManager, DistributedWebSocketManager>(); | ||
| } | ||
| else | ||
| { | ||
| // 단일 서버 환경 서비스 | ||
| services.AddSingleton<IMessageBroker, LocalMessageBroker>(); | ||
| services.AddSingleton<IWebSocketManager, WebSocketManager>(); |
There was a problem hiding this comment.
치명적: DistributedMessageBroker ↔ DistributedWebSocketManager 생성자 순환 의존성으로 DI 해석 실패
현재 싱글톤 등록에서 다음 순환이 발생합니다:
- DistributedMessageBroker → IWebSocketManager
- DistributedWebSocketManager → IMessageBroker
MS.DI는 생성자 주입 순환을 허용하지 않아 런타임에 InvalidOperationException이 납니다.
권장 수정(간단/국소 변경):
- DistributedWebSocketManager에서 IMessageBroker 직접 주입을 제거하고 IServiceProvider를 주입해 필요 시 지연 조회하도록 변경합니다. 그 후 분산 브로커인 경우에만 Subscribe/Unsubscribe를 호출하세요.
예시 (다른 파일 수정):
// DistributedWebSocketManager.cs
using Microsoft.Extensions.DependencyInjection; // GetRequiredService 용
public class DistributedWebSocketManager : IWebSocketManager
{
private readonly ILogger<DistributedWebSocketManager> _logger;
private readonly IConnectionRegistry _connectionRegistry;
private readonly ISessionStorage _sessionStorage;
private readonly IServiceProvider _serviceProvider; // 변경
public DistributedWebSocketManager(
ILogger<DistributedWebSocketManager> logger,
IConnectionRegistry connectionRegistry,
ISessionStorage sessionStorage,
IServiceProvider serviceProvider) // 변경
{
_logger = logger;
_connectionRegistry = connectionRegistry;
_sessionStorage = sessionStorage;
_serviceProvider = serviceProvider;
}
public async Task<string> ConnectAsync(string userId)
{
...
var broker = _serviceProvider.GetService<IMessageBroker>();
if (broker is DistributedMessageBroker distributed)
{
await distributed.SubscribeToUserChannelAsync(userId);
}
return userId;
}
public async Task DisconnectAsync(string userId)
{
...
var broker = _serviceProvider.GetService<IMessageBroker>();
if (broker is DistributedMessageBroker distributed)
{
await distributed.UnsubscribeFromUserChannelAsync(userId);
}
}
}대안:
- Lazy/Func 기반 지연 주입 또는 Subscribe/Unsubscribe 전용 인터페이스(예: IUserChannelSubscriber)를 도입해 의존 방향을 단방향으로 정리.
이 수정이 반영되면 본 파일의 DI 등록은 그대로 유지해도 순환이 해소됩니다.
🤖 Prompt for AI Agents
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs lines 91-98: the
DI registrations trigger a constructor cycle between DistributedMessageBroker
and DistributedWebSocketManager (each injects the other's interface); to fix,
change DistributedWebSocketManager to stop taking IMessageBroker in its
constructor and instead accept IServiceProvider (or a
lazy/Func<IMessageBroker>), update its ConnectAsync/DisconnectAsync to resolve
IMessageBroker at call-time (e.g., serviceProvider.GetService<IMessageBroker>),
and only call Subscribe/Unsubscribe when the resolved broker is a
DistributedMessageBroker; this removes the circular constructor dependency while
keeping the service registrations unchanged.
| var server = _redis.GetServer(_redis.GetEndPoints().First()); | ||
| var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*"); | ||
| var sessions = new List<SessionInfo>(); | ||
|
|
||
| foreach (var key in keys) | ||
| { | ||
| var value = await _database.StringGetAsync(key); | ||
| if (value.HasValue) | ||
| { | ||
| var session = JsonSerializer.Deserialize<SessionInfo>(value!); | ||
| if (session != null) | ||
| { | ||
| sessions.Add(session); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
server.Keys + First()는 클러스터/샤드 환경에서 누락·병목 유발
- 첫 번째 endpoint만 SCAN하므로 클러스터/샤드 구성에서 일부 키가 누락됩니다.
- 전역 SCAN + per-key GET은 비용이 큽니다. 세션 인덱스(Set)로 전환해 SCARD/SMEMBERS+MGET을 사용하세요.
권장 수정(세션 인덱스 도입):
@@
- private const string SESSION_KEY_PREFIX = "session:user:";
+ private const string SESSION_KEY_PREFIX = "session:user:";
+ private const string SESSIONS_INDEX_KEY = "session:index:all";
@@ public async Task<SessionInfo> CreateAsync(SessionInfo session)
- await _database.StringSetAsync(key, json, SESSION_TTL);
+ await _database.StringSetAsync(key, json, SESSION_TTL);
+ await _database.SetAddAsync(SESSIONS_INDEX_KEY, session.SessionId);
@@ public async Task DeleteAsync(string sessionId)
- var deleted = await _database.KeyDeleteAsync(key);
+ var deleted = await _database.KeyDeleteAsync(key);
if (deleted)
{
_logger.LogInformation("세션 삭제 성공: {SessionId}", sessionId);
+ await _database.SetRemoveAsync(SESSIONS_INDEX_KEY, sessionId);
}
@@ public async Task<IEnumerable<SessionInfo>> GetAllAsync()
- var server = _redis.GetServer(_redis.GetEndPoints().First());
- var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*");
- var sessions = new List<SessionInfo>();
-
- foreach (var key in keys)
- {
- var value = await _database.StringGetAsync(key);
- if (value.HasValue)
- {
- var session = JsonSerializer.Deserialize<SessionInfo>(value!);
- if (session != null)
- {
- sessions.Add(session);
- }
- }
- }
+ var ids = await _database.SetMembersAsync(SESSIONS_INDEX_KEY);
+ if (ids.Length == 0)
+ return Enumerable.Empty<SessionInfo>();
+
+ var keys = ids
+ .Select(id => (RedisKey)(SESSION_KEY_PREFIX + (string)id))
+ .ToArray();
+ var values = await _database.StringGetAsync(keys);
+ var sessions = new List<SessionInfo>(values.Length);
+ foreach (var v in values)
+ {
+ if (!v.HasValue) continue;
+ var s = JsonSerializer.Deserialize<SessionInfo>(v.Span);
+ if (s != null) sessions.Add(s);
+ }
_logger.LogDebug("전체 세션 조회: {Count}개", sessions.Count);
return sessions;
@@ public Task<int> GetActiveSessionCountAsync()
- var server = _redis.GetServer(_redis.GetEndPoints().First());
- var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*");
- var count = keys.Count();
+ var count = await _database.SetLengthAsync(SESSIONS_INDEX_KEY);
_logger.LogDebug("활성 세션 수: {Count}", count);
- return Task.FromResult(count);
+ return Task.FromResult((int)count);단기 대응(인덱스 도입 전 임시 방편):
- var server = _redis.GetServer(_redis.GetEndPoints().First());
- var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*");
+ var endpoints = _redis.GetEndPoints();
+ var keys = endpoints.SelectMany(ep =>
+ _redis.GetServer(ep).Keys(database: _database.Database, pattern: SESSION_KEY_PREFIX + "*", pageSize: 1000));Also applies to: 163-166
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the file and print its path
rg -n --hidden --glob '!**/node_modules/**' "RedisSessionStorage.cs" || true
# Print the file with line numbers for inspection
sed -n '1,240p' ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs || trueLength of output: 5568
클러스터/샤드 환경에서 server.Keys 사용은 키 누락·병목을 유발 — 긴급 수정 필요
GetServer(GetEndPoints().First())로 첫 엔드포인트만 스캔하며 server.Keys + per-key GET은 비용이 큽니다. 수정 필요.
- 위치: ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs — GetAllAsync, GetActiveSessionCountAsync (CreateAsync / DeleteAsync도 인덱스 유지 변경 필요)
- 권장 변경: 세션 인덱스(SESSIONS_INDEX_KEY) 도입 — CreateAsync: SetAddAsync(sessionId), DeleteAsync: SetRemoveAsync(sessionId), GetAllAsync: SetMembersAsync → RedisKey[]로 StringGetAsync(배치), GetActiveSessionCountAsync: SetLengthAsync
- 임시 대책(인덱스 도입 전): 모든 엔드포인트 순회하여 keys 조회 (endpoints.SelectMany(ep => _redis.GetServer(ep).Keys(..., pageSize: 1000)))
🤖 Prompt for AI Agents
In ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs around
lines 52-67, the code uses
_redis.GetServer(_redis.GetEndPoints().First()).Keys(...) and per-key
StringGetAsync which misses keys in cluster/sharded setups and causes
performance bottlenecks; replace this by introducing a session index key (e.g.,
SESSIONS_INDEX_KEY) and update CreateAsync to call SetAddAsync(sessionId) and
DeleteAsync to call SetRemoveAsync(sessionId); change GetAllAsync to call
SetMembersAsync(SESSIONS_INDEX_KEY) to get RedisKey[] and then perform a batched
StringGetAsync for those keys, and change GetActiveSessionCountAsync to use
SetLengthAsync(SESSIONS_INDEX_KEY); as a short-term mitigation before index
rollout, iterate all endpoints (endpoints.SelectMany(ep =>
_redis.GetServer(ep).Keys(..., pageSize: 1000))) to aggregate keys from every
server.
ImGdevel
changed the title
This reverts commit d8455dd.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (12)
ProjectVG.Application/Services/Chat/ChatService.cs (5)
10-10: 미사용 네임스페이스 — 제거 권고IWebSocketManager가 본 클래스에서 사용되지 않아 using이 불필요합니다.
-using ProjectVG.Application.Services.WebSocket;
22-22: 미사용 필드 추가됨 — 제거 또는 IMessageBroker로 전환주입된 IWebSocketManager는 어디에서도 사용되지 않습니다. 본 PR 방향(브로커 경유 전송)과도 상충되어 보입니다. 제거하거나 실제로 필요하다면 IMessageBroker로 교체하세요.
- private readonly IWebSocketManager _webSocketManager;
58-58: 미사용 필드 할당 — 제거필드 자체가 불필요하므로 할당도 제거하세요.
- _webSocketManager = webSocketManager;
74-75: 검증 로그 좋습니다. chatId 상관관계 키 추가 제안추적성 향상을 위해 command.Id를 함께 남기면 유용합니다(현재 레벨 Debug 적절).
- _logger.LogDebug("[채팅서비스] 요청 검증 완료: UserId={UserId}, CharacterId={CharacterId}", - command.UserId, command.CharacterId); + _logger.LogDebug("[채팅서비스] 요청 검증 완료: ChatId={ChatId}, UserId={UserId}, CharacterId={CharacterId}", + command.Id, command.UserId, command.CharacterId);
31-34: 주입된 필드와 스코프 Resolve 혼재 — 한 가지 방식으로 통일_resultProcessor / _chatFailureHandler를 생성자에서 주입하지만, 실제 사용은 스코프에서 다시 Resolve하고 있어 주입 필드는 데드 코드입니다. 배경 작업(Task.Run)에서 스코프 분리를 유지하려면 “스코프 Resolve 방식”으로 통일하고 불필요한 주입을 제거하세요.
- private readonly ChatResultProcessor _resultProcessor; - - private readonly ChatFailureHandler _chatFailureHandler; + // ChatResultProcessor / ChatFailureHandler는 처리 스코프에서 Resolve @@ - ICostTrackingDecorator<ChatTTSProcessor> ttsProcessor, - ChatResultProcessor resultProcessor, + ICostTrackingDecorator<ChatTTSProcessor> ttsProcessor, @@ - ChatFailureHandler chatFailureHandler + ChatFailureHandler chatFailureHandler ) { @@ - _ttsProcessor = ttsProcessor; - _resultProcessor = resultProcessor; - _chatFailureHandler = chatFailureHandler; + _ttsProcessor = ttsProcessor;참고: 위 diff는 두 필드/할당 제거를 보여주며, 이미 스코프에서 result/failure 핸들러를 Resolve하고 있으므로 추가 변경은 필요 없습니다.
Also applies to: 119-127
ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (1)
5-5: IWebSocketManager 주입 OK. ISessionStorage는 현재 미사용 → 의존성 제거 권장세션 검증이 WebSocket 기반으로 전환되었으므로, ISessionStorage 필드/생성자 주입/할당은 제거해 DI를 단순화하는 것이 좋습니다.
DI 등록부에서도 ChatRequestValidator 생성자 시그니처를 맞춰 주입 목록을 갱신해 주세요.
권장 수정안:
-using ProjectVG.Infrastructure.Persistence.Session; +// using 제거 (더 이상 필요 없으면) - private readonly ISessionStorage _sessionStorage; private readonly IWebSocketManager _webSocketManager; - public ChatRequestValidator( - ISessionStorage sessionStorage, + public ChatRequestValidator( IWebSocketManager webSocketManager, IUserService userService, ICharacterService characterService, ICreditManagementService tokenManagementService, ILogger<ChatRequestValidator> logger) { - _sessionStorage = sessionStorage; _webSocketManager = webSocketManager;Also applies to: 14-15, 25-26, 31-33
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (4)
66-110: 서버 조회 후에도 사용자 채널로 퍼블리시: TOCTOU로 인한 메시지 손실 가능성.사용자 채널은 연결/해제에 따라 동적으로 구독되므로, 매핑 존재(GetUserServerAsync) 시점과 실제 구독 상태 사이에 갭이 있을 수 있습니다. 안정성을 높이려면 항상 서버 채널(server:{serverId})로 라우팅하고 서버 수신 핸들러에서 로컬 전송하도록 변경하는 것을 권장합니다. 현재 OnServerMessageReceived TODO가 있어 당장 전환이 어렵다면, 재시도/백오프 또는 퍼블리시 이중화(서버 채널 + 사용자 채널)를 검토하세요.
278-301: 고빈도 로그 레벨 하향 필요.메시지 송수신 단위의 LogInformation은 운영 환경에서 과도합니다. Debug/Trace로 하향해 로그폭주를 방지하세요.
Also applies to: 70-81, 98-104, 191-206
314-326: Dispose 시 사용자 채널 정리 누락 가능성.서버/브로드캐스트만 해제합니다. 프로세스 종료 시 구독이 연결과 함께 정리되긴 하지만, 명시적으로 사용자 채널도 추적/해제하면 깔끔합니다(예: HashSet으로 구독 채널 트래킹).
34-41: 미사용 필드 정리._subscriber만 사용하며 _redis 필드는 더 이상 사용되지 않습니다. 제거해 간결성을 유지하세요.
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (2)
45-56: 구체 타입 캐스팅 제거: 분산 브로커 전용 인터페이스로 의존 역전.GetService() as DistributedMessageBroker 캐스팅은 결합을 키웁니다. 사용자 채널 구독/해제를 포함한 IDistributedMessageBroker(또는 IMessageBroker에 메서드 확장)를 도입해 의존성을 인터페이스로 전환하세요.
예시:
public interface IDistributedMessageBroker : IMessageBroker { Task SubscribeToUserChannelAsync(string userId); Task UnsubscribeFromUserChannelAsync(string userId); } // 사용처 var messageBroker = _serviceProvider.GetService<IDistributedMessageBroker>(); if (messageBroker != null) { await messageBroker.SubscribeToUserChannelAsync(userId); }Also applies to: 102-109
83-94: 원격 사용자 전송 경로 미연결 안내.이 매니저는 로컬 연결로만 송신합니다. 분산 환경에서 원격 사용자 송신은 IMessageBroker 경로만 사용됨을 명확히 하거나, 필요 시 메시지 브로커 경유 옵션을 추가하세요.
Also applies to: 70-81
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
ProjectVG.Api/appsettings.json(1 hunks)ProjectVG.Application/Services/Chat/ChatService.cs(5 hunks)ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs(4 hunks)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs(1 hunks)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs(1 hunks)ProjectVG.Common/Constants/ErrorCodes.cs(2 hunks)ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs(2 hunks)docker-compose.yml(3 hunks)test-clients/ai-chat-client/script.js(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- test-clients/ai-chat-client/script.js
- docker-compose.yml
- ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs
🧰 Additional context used
🧬 Code graph analysis (4)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (4)
ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (1)
GetServerId(125-128)ProjectVG.Application/Services/Server/IServerRegistrationService.cs (6)
GetServerId(25-25)Task(10-10)Task(15-15)Task(20-20)Task(30-30)Task(35-35)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (6)
Task(34-61)Task(63-68)Task(70-81)Task(83-94)Task(96-112)Task(119-141)ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs (6)
BrokerMessage(5-88)BrokerMessage(16-29)BrokerMessage(31-44)BrokerMessage(46-58)BrokerMessage(77-87)ToJson(72-75)
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (5)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (8)
Task(66-110)Task(112-126)Task(128-143)Task(148-164)Task(169-185)Task(270-312)DistributedMessageBroker(13-327)DistributedMessageBroker(28-42)ProjectVG.Common/Models/Session/SessionInfo.cs (1)
SessionInfo(3-9)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (1)
IsSessionActive(25-25)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (1)
IsSessionActive(74-77)ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (2)
RedisSessionStorage(8-222)RedisSessionStorage(17-22)
ProjectVG.Application/Services/Chat/ChatService.cs (1)
test-loadtest/chat-loadtest.js (1)
WebSocket(9-9)
ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (5)
ProjectVG.Application/Services/Chat/ChatService.cs (3)
Task(69-84)Task(90-107)Task(112-133)ProjectVG.Api/Middleware/WebSocketMiddleware.cs (4)
Task(31-53)Task(94-104)Task(109-205)Guid(58-74)ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (1)
IsSessionActive(114-117)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs (1)
IsSessionActive(25-25)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs (1)
IsSessionActive(74-77)
🪛 GitHub Check: Build & Test
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs
[warning] 318-318:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 249-249:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
[warning] 227-227:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
[warning] 174-174:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 153-153:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 135-135:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 117-117:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 101-101:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 53-53:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 50-50:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
🔇 Additional comments (7)
ProjectVG.Application/Services/Chat/ChatService.cs (1)
41-41: 제거 금지 — IWebSocketManager는 DI에 등록되어 있고 여러 곳에서 사용됩니다ApplicationServiceCollectionExtensions.cs(services.AddSingleton<IWebSocketManager, ...>)에 등록되어 있으며 WebSocketMiddleware.cs, ChatResultProcessor.cs, ChatFailureHandler.cs, LocalMessageBroker.cs, ChatRequestValidator.cs 및 테스트(ProjectVG.Tests/.../ChatServiceSimpleTests.cs)에서 참조됩니다. 생성자 매개변수 제거 제안은 부적절합니다.
Likely an incorrect or invalid review comment.
ProjectVG.Common/Constants/ErrorCodes.cs (2)
183-183: 오류 메시지 매핑 추가 LGTM문구/톤이 기존 매핑과 일관적입니다.
87-87: 새 오류 코드 추가 LGTM — enum 순서(ordinal) 의존성 없음 확인리포지토리 전역 검색 결과 (정수 캐스팅: (int)ErrorCode, EF 변환: HasConversion<int|ErrorCode 등) 관련 패턴이 발견되지 않았고, 예외 처리/응답에서는 ErrorCode.GetCode()/ToString()로 문자열을 사용하므로 ordinal 변경의 영향은 없습니다.
ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (3)
76-78: 문서화 업데이트 LGTM역할 전환(분산 환경의 WebSocket 세션 검증)이 명확합니다.
81-91: WebSocket 활성 세션 확인 흐름 LGTM활성 연결 미존재 시 WEBSOCKET_SESSION_REQUIRED로 단락(return) 처리하는 판단 적절합니다. 클라이언트 UX 관점에서도 명확한 에러입니다.
95-105: 검증 완료 — 컴파일 오류 아님: UpdateSessionHeartbeatAsync가 인터페이스와 구현에 존재합니다ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs에 Task UpdateSessionHeartbeatAsync(string userId)가 선언되어 있고 WebSocketManager.cs(라인 ~79) 및 DistributedWebSocketManager.cs(라인 ~119)에 구현이 있으므로 해당 호출은 유효합니다.
Likely an incorrect or invalid review comment.
ProjectVG.Application/Services/WebSocket/DistributedWebSocketManager.cs (1)
7-7: 레이어링 위반: Application에서 Infrastructure 직접 참조.이전 리뷰와 동일 이슈입니다. Heartbeat를 위한 타입 체크를 인터페이스로 승격해 Infrastructure 의존을 제거하세요.
예시:
// Application 계층에 인터페이스 추가 namespace ProjectVG.Application.Services.Session { public interface IHeartbeatSessionStorage { Task<bool> HeartbeatAsync(string sessionId); } }-using ProjectVG.Infrastructure.Persistence.Session; ... - if (_sessionStorage is ProjectVG.Infrastructure.Persistence.Session.RedisSessionStorage redisStorage) - { - await redisStorage.HeartbeatAsync(userId); - } + if (_sessionStorage is IHeartbeatSessionStorage heartbeatStorage) + { + await heartbeatStorage.HeartbeatAsync(userId); + }등록은 Composition Root에서 RedisSessionStorage : ISessionStorage, IHeartbeatSessionStorage로 바인딩.
Also applies to: 125-134
| "DistributedSystem": { | ||
| "Enabled": true, | ||
| "ServerId": "api-server-001", | ||
| "HeartbeatIntervalSeconds": 30, | ||
| "CleanupIntervalMinutes": 5, | ||
| "ServerTimeoutMinutes": 2 | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
ServerId 고정값은 인스턴스 충돌 유발 — 인스턴스별 유일값 보장 필요
"api-server-001" 고정은 다중 인스턴스에서 충돌/세션 오염을 유발합니다. 권장:
- ServerId를 환경변수로 주입(예: 컨테이너 인스턴스 ID/호스트명 기반).
- 코드에서 미지정/공백일 때 머신명+GUID 등으로 자동 생성.
또한 간격/단위 정합성 확인:
HeartbeatIntervalSeconds=30,ServerTimeoutMinutes=2,CleanupIntervalMinutes=5조합은 최대 5분까지 정리 지연이 발생합니다. 타임아웃(2분) ≤ 정리주기(5분)이면 지연이 큽니다. 정리주기를 더 짧게 권장합니다.
빠른 개선안(정리 주기 단축):
- "CleanupIntervalMinutes": 5,
+ "CleanupIntervalMinutes": 1,추가 권장(선택):
- 단위 혼용(Seconds/Minutes)으로 혼동 여지 → 동일 단위로 통일하거나 옵션 명확화.
- 환경/클러스터 분리를 위한 ChannelPrefix/Namespace 추가로 키/채널 충돌 방지.
중복 ServerId 사용 가능성 점검:
🏁 Script executed:
#!/bin/bash
rg -n -C2 '"ServerId"\s*:\s*"' --hiddenLength of output: 1942
ServerId 하드코딩 제거 — 인스턴스별 유일값 필요
ProjectVG.Api/appsettings.json에 "ServerId": "api-server-001"이 하드코딩되어 있어 다중 인스턴스에서 충돌/세션 오염 발생 가능. 문서에도 동일 예시가 존재함.
- 조치: ServerId를 환경변수로 주입(컨테이너 인스턴스 ID/호스트명 또는 런타임 생성 GUID). 미지정/빈값일 경우 머신명+GUID로 자동 생성.
- 타이밍 정합성: HeartbeatIntervalSeconds=30, ServerTimeoutMinutes=2, CleanupIntervalMinutes=5 조합은 정리 지연을 초래함 — Cleanup 주기 단축(예: 1분) 또는 단위 통일 권장.
빠른 개선안:
- "CleanupIntervalMinutes": 5,
+ "CleanupIntervalMinutes": 1,수정 위치: ProjectVG.Api/appsettings.json (약 lines 14–18); 문서 예시: docs/distributed-system/README.md, docs/architecture/chat-system-flow.md.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "DistributedSystem": { | |
| "Enabled": true, | |
| "ServerId": "api-server-001", | |
| "HeartbeatIntervalSeconds": 30, | |
| "CleanupIntervalMinutes": 5, | |
| "ServerTimeoutMinutes": 2 | |
| }, | |
| "DistributedSystem": { | |
| "Enabled": true, | |
| "ServerId": "api-server-001", | |
| "HeartbeatIntervalSeconds": 30, | |
| "CleanupIntervalMinutes": 1, | |
| "ServerTimeoutMinutes": 2 | |
| }, |
🤖 Prompt for AI Agents
In ProjectVG.Api/appsettings.json around lines 14 to 20, remove the hardcoded
"ServerId": "api-server-001" and instead read ServerId from an environment
variable (e.g., DISTRIBUTED_SERVER_ID); if the env var is missing or empty,
generate a fallback value at runtime using the machine hostname concatenated
with a GUID to guarantee per-instance uniqueness; update the configuration
loading logic to accept the env var and runtime fallback and persist no
hardcoded value in appsettings.json; also adjust CleanupIntervalMinutes to a
shorter/default value (e.g., 1) or unify timing units with
HeartbeatIntervalSeconds/ServerTimeoutMinutes to avoid delayed cleanup, and
update docs (docs/distributed-system/README.md and
docs/architecture/chat-system-flow.md) to show using the env var or
runtime-generated ServerId.
| "LLM": { | ||
| "BaseUrl": "http://localhost:7930" | ||
| }, | ||
| "MEMORY": { | ||
| "BaseUrl": "http://localhost:7940" | ||
| }, | ||
| "TTS": { | ||
| "BaseUrl": "https://supertoneapi.com" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
외부 서비스 BaseUrl만으로는 운영 리스크 — 타임아웃/재시도 등 구성 노출 권장
LLM/MEMORY/TTS 섹션에 최소 다음을 함께 노출하면 운영 안정성이 향상됩니다:
- 요청 타임아웃(초), 재시도/백오프, 회로차단(서킷브레이커) 스위치
- 베이스 URL 트레일링 슬래시 규칙 통일(클라이언트에서 중복/누락 방지)
- 인증 토큰/키는 비밀 주입으로 처리
예시(권장 키; 실제 반영은 코드 지원 필요):
- TimeoutSeconds, RetryCount, RetryBackoffMillis, CircuitBreakerEnabled
BaseUrl 사용부에서 슬래시 정규화/타임아웃이 적용되는지 확인:
🏁 Script executed:
#!/bin/bash
rg -n -C3 -i '(LLM|MEMORY|TTS).*BaseUrl|HttpClient|Timeout|Circuit|Retry' --type cs --hiddenLength of output: 35983
운영 리스크: LLM/MEMORY/TTS 외부 클라이언트 — 타임아웃/재시도/회로차단 구성 노출 및 BaseUrl 슬래시 정규화 필요
- 검증 결과: ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs에서 BaseAddress만 설정(client.BaseAddress = new Uri(...))하고 재시도/백오프/서킷브레이커/타임아웃 정책 미적용.
- LLMClient는 생성자에서 _httpClient.Timeout = TimeSpan.FromSeconds(30)로 하드코딩됨 (ProjectVG.Infrastructure/Integrations/LLMClient/LLMClient.cs) — 타임아웃을 구성으로 이동 필요.
- Memory/TTS 클라이언트에는 명시적 타임아웃·재시도·서킷 적용 없음 (ProjectVG.Infrastructure/Integrations/MemoryClient/VectorMemoryClient.cs, ProjectVG.Infrastructure/Integrations/TextToSpeechClient/TextToSpeechClient.cs).
- 경로 결합 불일치: LLMClient.PostAsync("api/v1/chat") (선행 슬래시 없음) vs Memory/TTS는 "/api/…" (선행 슬래시 있음) — BaseUrl에 경로/트레일링 슬래시가 포함되면 URI 결합 오류 가능. BaseUrl 정규화(슬래시 규칙 통일) 또는 클라이언트 경로 결합 규칙 통일 필요.
- 권장 조치: appsettings.json에 TimeoutSeconds/RetryCount/RetryBackoffMillis/CircuitBreakerEnabled 등 노출, InfrastructureServiceCollectionExtensions.cs의 AddHttpClient에 Polly 기반 재시도·백오프·서킷·타임아웃 정책 추가(또는 HttpClientFactory 설정으로 중앙관리), LLMClient 내부 하드코딩 제거 및 BaseUrl 정규화/검증 로직 추가. 인증 키는 시크릿 주입(환경변수/시크릿스토어) 유지.
| "BaseUrl": "https://supertoneapi.com" | ||
| }, | ||
| "ConnectionStrings": { | ||
| "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;User Id=sa;Password=ProjectVG123!;TrustServerCertificate=true;MultipleActiveResultSets=true", |
There was a problem hiding this comment.
🧩 Analysis chain
심각: DB 자격증명(비밀번호) 커밋됨 — 즉시 제거 및 비밀 회수 필요
sa 계정 비밀번호가 평문으로 커밋되었습니다. 이는 치명적 보안사고로 이어질 수 있습니다. 기본 원칙: 비밀번호는 절대 저장소에 두지 말고 환경변수/비밀관리(User-Secrets, Vault 등)로 주입하세요.
권고 조치:
- 비밀번호 교체(회수) 및 DB 사용자 최소권한 계정으로 전환.
- 저장소에서 값 제거(Git 이력도 정리 필요).
- 환경별 오버라이드(appsettings.{Environment}.json, 환경변수)로 주입.
즉시 적용 가능한 임시 수정(기본값에서 비밀번호 제거; 실제 값은 환경변수로 오버라이드):
- "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;User Id=sa;Password=ProjectVG123!;TrustServerCertificate=true;MultipleActiveResultSets=true",
+ "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;TrustServerCertificate=true;MultipleActiveResultSets=true",비밀 노출 여부 점검 스크립트:
🏁 Script executed:
#!/bin/bash
# 민감 문자열 및 연결문자열 유사 패턴 탐지
rg -n -C1 -i '(Password=|User Id=sa|ProjectVG123!)|DefaultConnection' --hiddenLength of output: 155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for 'DefaultConnection' (fixed string)..."
rg -nF --hidden --no-ignore 'DefaultConnection' -S || true
echo
echo "Searching for 'Password=' (fixed string)..."
rg -nF --hidden --no-ignore 'Password=' -S || true
echo
echo "Searching for 'User Id=sa' (fixed string)..."
rg -nF --hidden --no-ignore 'User Id=sa' -S || true
echo
echo "Showing ProjectVG.Api/appsettings.json (if present):"
if [ -f ProjectVG.Api/appsettings.json ]; then
nl -ba ProjectVG.Api/appsettings.json | sed -n '1,200p'
else
echo "File not found: ProjectVG.Api/appsettings.json"
fiLength of output: 3207
즉시 조치 필요: 평문 DB 비밀번호(sa:ProjectVG123!)가 커밋됨 — 즉시 제거·회수 및 이력 정리
다음 파일에서 평문 연결문자열 확인(수정 필요):
- ProjectVG.Api/appsettings.json:31
- ProjectVG.Api/appsettings.Development.json:11
- ProjectVG.Infrastructure/Persistence/EfCore/Data/DesignTimeDbContextFactory.cs:38
권장 즉시 조치:
- DB 비밀번호(해당 sa 계정) 즉시 교체 및 해당 자격증명 폐기 → 최소 권한 계정 사용.
- 저장소에서 민감값 제거 후 Git 이력에서 완전 삭제(예: git filter-repo 또는 BFG). 단순 커밋 삭제만으로는 불충분.
- 코드/설정에서 평문 제거 — 연결문자열은 환경변수 또는 시크릿스토어(User-Secrets/HashiCorp Vault/GitHub Secrets 등)로 주입.
- DesignTimeDbContextFactory.cs 내 하드코딩된 연결문자열을 제거하고 구성(Configuration) 또는 환경변수로 대체.
임시 패치(앱설정에서 비밀번호 제거 — 실제 값은 환경변수로 주입):
- "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;User Id=sa;Password=ProjectVG123!;TrustServerCertificate=true;MultipleActiveResultSets=true",
+ "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;TrustServerCertificate=true;MultipleActiveResultSets=true",추가 확인: .env.example 및 문서에 예시 연결문자열 존재(민감값 포함 여부 점검). 모든 변경 후 CI/CD 및 배포 비밀도 갱신.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;User Id=sa;Password=ProjectVG123!;TrustServerCertificate=true;MultipleActiveResultSets=true", | |
| "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;TrustServerCertificate=true;MultipleActiveResultSets=true", |
🤖 Prompt for AI Agents
In ProjectVG.Api/appsettings.json around line 31 (and also review
ProjectVG.Api/appsettings.Development.json line 11 and
ProjectVG.Infrastructure/Persistence/EfCore/Data/DesignTimeDbContextFactory.cs
line 38), remove the hardcoded plaintext sa connection string and replace it
with a placeholder that reads the connection string (or just the DB password)
from an environment variable or secret store; update DesignTimeDbContextFactory
to obtain configuration from IConfiguration or
Environment.GetEnvironmentVariable rather than a hardcoded literal; rotate the
exposed sa credential immediately (create a new least-privilege DB user and
revoke/disable the compromised account) and ensure CI/CD secrets are updated;
then purge the secret from Git history using git filter-repo or BFG and verify
.env.example/docs contain no real secrets.
| }, | ||
| "ConnectionStrings": { | ||
| "DefaultConnection": "Server=localhost,1433;Database=ProjectVG;User Id=sa;Password=ProjectVG123!;TrustServerCertificate=true;MultipleActiveResultSets=true", | ||
| "Redis": "projectvg-redis:6379" |
There was a problem hiding this comment.
🧩 Analysis chain
Redis 보안 설정 누락(인증/TLS) — 프로덕션 대비 강화 필요
현재 "projectvg-redis:6379"로 인증/암호화 없이 접속합니다. 운영환경에서는 최소한 다음을 보장하세요:
- 인증 비밀번호 사용(password) 및 주입은 환경변수/비밀관리로.
- TLS 사용(예:
ssl=true등 라이브러리 호환 옵션). - 타임아웃/재시도/풀 크기 옵션 튜닝.
개발/로컬은 유지하되, appsettings.Production.json 또는 환경변수로 보안 옵션을 강제하는 구성을 추가해 주세요.
비슷한 설정이 다른 파일에 하드코딩되지 않았는지 점검:
🏁 Script executed:
#!/bin/bash
rg -n -C2 -i 'redis.*(password|ssl|rediss|:6379)' --hiddenLength of output: 1835
Redis 보안(인증/TLS) 미설정 — 프로덕션 적용 전 즉시 수정 필요
문제: ProjectVG.Api/appsettings.json에 인증/암호화 없이 Redis가 하드코딩되어 있습니다:
"Redis": "projectvg-redis:6379"
조치(우선순위)
- Redis 연결 문자열에서 비밀번호 사용(ACL) — 민감값은 환경변수 또는 시크릿 매니저로 주입.
- TLS 활성화(예: rediss:// 또는 클라이언트 옵션 ssl=true 등) 및 클라이언트 라이브러리 설정 확인.
- 프로덕션 전용 구성 파일(appsettings.Production.json) 또는 환경변수로 보안 옵션 강제.
- 문서/배포 예시 업데이트(docs/deployment/ci-cd-setup.md의 REDIS_CONNECTION_STRING 예시 점검).
- 코드 검증: ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs가 안전하게 구성된 IConnectionMultiplexer를 사용하도록 확인(구성 주입 경로 점검).
ProjectVG.Api/appsettings.json, docs/deployment/ci-cd-setup.md, ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs에서 우선 수정 필요.
🤖 Prompt for AI Agents
In ProjectVG.Api/appsettings.json around line 32, the Redis connection is
hardcoded without auth/TLS ("Redis": "projectvg-redis:6379"); update
configuration to require a secured connection string (use password/ACL and TLS
scheme like rediss:// or ssl=true) sourced from environment variables or a
secrets manager (move this value into appsettings.Production.json and/or read
from ASPNETCORE configuration for production), update
docs/deployment/ci-cd-setup.md to show REDIS_CONNECTION_STRING example with
credentials and TLS, and verify
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs uses the
injected connection string to configure IConnectionMultiplexer with password and
SSL options (fail fast if missing) so production always uses
authenticated+encrypted Redis.
| { | ||
| // 이 서버로 오는 메시지 구독 | ||
| var serverChannel = $"{SERVER_CHANNEL_PREFIX}:{_serverId}"; | ||
| _subscriber.Subscribe(serverChannel, OnServerMessageReceived); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
RedisChannel 암시적 변환 사용 중단 경고 해결: Literal 사용으로 교체하세요.
StackExchange.Redis v2 권고에 따라 채널 지정 시 RedisChannel.Literal(...)을 사용해야 합니다. 아래 패치를 적용하면 모든 경고가 제거됩니다.
- _subscriber.Subscribe(serverChannel, OnServerMessageReceived);
+ _subscriber.Subscribe(RedisChannel.Literal(serverChannel), OnServerMessageReceived);
- _subscriber.Subscribe(BROADCAST_CHANNEL, OnBroadcastMessageReceived);
+ _subscriber.Subscribe(RedisChannel.Literal(BROADCAST_CHANNEL), OnBroadcastMessageReceived);
- await _subscriber.PublishAsync(userChannel, brokerMessage.ToJson());
+ await _subscriber.PublishAsync(RedisChannel.Literal(userChannel), brokerMessage.ToJson());
- await _subscriber.PublishAsync(BROADCAST_CHANNEL, brokerMessage.ToJson());
+ await _subscriber.PublishAsync(RedisChannel.Literal(BROADCAST_CHANNEL), brokerMessage.ToJson());
- await _subscriber.PublishAsync(serverChannel, brokerMessage.ToJson());
+ await _subscriber.PublishAsync(RedisChannel.Literal(serverChannel), brokerMessage.ToJson());
- await _subscriber.SubscribeAsync(userChannel, OnUserMessageReceived);
+ await _subscriber.SubscribeAsync(RedisChannel.Literal(userChannel), OnUserMessageReceived);
- await _subscriber.UnsubscribeAsync(userChannel);
+ await _subscriber.UnsubscribeAsync(RedisChannel.Literal(userChannel));
- _subscriber?.Unsubscribe($"{SERVER_CHANNEL_PREFIX}:{_serverId}");
- _subscriber?.Unsubscribe(BROADCAST_CHANNEL);
+ _subscriber?.Unsubscribe(RedisChannel.Literal($"{SERVER_CHANNEL_PREFIX}:{_serverId}"));
+ _subscriber?.Unsubscribe(RedisChannel.Literal(BROADCAST_CHANNEL));Also applies to: 53-53, 101-101, 117-117, 135-135, 153-153, 174-174, 318-319
🧰 Tools
🪛 GitHub Check: Build & Test
[warning] 50-50:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
🤖 Prompt for AI Agents
In ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs
around lines 50, 53, 101, 117, 135, 153, 174 and 318-319, the code is passing
string literals implicitly to StackExchange.Redis methods which triggers the
"implicit RedisChannel conversion" warnings; replace each string channel
argument with RedisChannel.Literal(channelString) (e.g.,
RedisChannel.Literal(serverChannel)) for Subscribe/Publish/other channel calls
so the explicit literal API is used and warnings are removed; keep existing
callback/handler parameters unchanged and ensure the StackExchange.Redis
namespace is in scope.
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs
Show resolved
Hide resolved
| private async void OnServerMessageReceived(RedisChannel channel, RedisValue message) | ||
| { | ||
| try | ||
| { | ||
| var brokerMessage = BrokerMessage.FromJson(message!); | ||
| if (brokerMessage == null) | ||
| { | ||
| _logger.LogWarning("잘못된 서버 메시지 형식: {Message}", message); | ||
| return; | ||
| } | ||
|
|
||
| // 서버별 메시지 처리 로직 | ||
| _logger.LogDebug("서버 메시지 수신: {MessageType}", brokerMessage.MessageType); | ||
|
|
||
| // TODO: 서버별 메시지 타입에 따른 처리 로직 구현 | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "서버 메시지 수신 처리 중 오류: {Channel}", channel); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
async void 메서드에 await 없음: 시그니처를 동기 void로 변경하세요.
경고대로 실제 비동기 작업이 없으므로 async 제거가 안전합니다. 예외 전달/관리를 명확히 합니다.
- private async void OnServerMessageReceived(RedisChannel channel, RedisValue message)
+ private void OnServerMessageReceived(RedisChannel channel, RedisValue message)
{
try
{
var brokerMessage = BrokerMessage.FromJson(message!);
if (brokerMessage == null)
{
_logger.LogWarning("잘못된 서버 메시지 형식: {Message}", message);
return;
}
// 서버별 메시지 처리 로직
_logger.LogDebug("서버 메시지 수신: {MessageType}", brokerMessage.MessageType);
// TODO: 서버별 메시지 타입에 따른 처리 로직 구현
}
catch (Exception ex)
{
_logger.LogError(ex, "서버 메시지 수신 처리 중 오류: {Channel}", channel);
}
}
- private async void OnBroadcastMessageReceived(RedisChannel channel, RedisValue message)
+ private void OnBroadcastMessageReceived(RedisChannel channel, RedisValue message)
{
try
{
var brokerMessage = BrokerMessage.FromJson(message!);
if (brokerMessage == null || brokerMessage.SourceServerId == _serverId)
{
// 자신이 보낸 메시지는 무시
return;
}
// 현재 서버에 연결된 모든 사용자에게 브로드캐스트
// TODO: IConnectionRegistry에서 모든 활성 사용자 목록을 가져와서 전송
_logger.LogDebug("브로드캐스트 메시지 수신: {MessageType}", brokerMessage.MessageType);
}
catch (Exception ex)
{
_logger.LogError(ex, "브로드캐스트 메시지 수신 처리 중 오류");
}
}Also applies to: 249-268
🧰 Tools
🪛 GitHub Check: Build & Test
[warning] 227-227:
This async method lacks 'await' operators and will run synchronously. Consider using the 'await' operator to await non-blocking API calls, or 'await Task.Run(...)' to do CPU-bound work on a background thread.
🤖 Prompt for AI Agents
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs around
lines 227-247 and 249-268: both OnServerMessageReceived (and the other similar
handler) are declared async void but contain no await; change the signature to
synchronous void (remove async) so exceptions are propagated/handled predictably
and the compiler warning is resolved, keep the existing try/catch for error
logging; if you later add real asynchronous work, convert the method to return
Task and await inside and update any caller/subscriber to accept an async Task
handler.
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ProjectVG.Application/Services/Chat/Handlers/ChatFailureHandler.cs (1)
26-28: 로그 템플릿/매개변수 불일치메시지 템플릿은 UserId를 가리키지만 실제로 RequestId를 전달하고 있습니다. 혼동 방지를 위해 필드명과 값 매칭을 수정하세요.
- _logger.LogError(ex, "오류 메시지 전송 실패: 세션 {UserId}", context.RequestId); + _logger.LogError(ex, "오류 메시지 전송 실패: 요청 {RequestId}, 사용자 {UserId}", context.RequestId, context.UserId);
🧹 Nitpick comments (18)
ProjectVG.Application/Services/Chat/Handlers/ChatFailureHandler.cs (1)
23-25: 실패 응답에 상관관계 식별자(requestId) 포함 및 메시지 구조화 제안클라이언트 디버깅/재시도 용이성을 위해 requestId를 payload에 포함하고 간단한 에러 메시지를 전달하는 편이 좋습니다.
- var errorResponse = new WebSocketMessage("fail", ""); - await _messageBroker.SendToUserAsync(context.UserId.ToString(), errorResponse); + var requestId = context.RequestId.ToString(); + var userId = context.UserId.ToString(); + var errorPayload = new { requestId, message = "채팅 처리 중 오류가 발생했습니다." }; + var errorResponse = new WebSocketMessage("fail", errorPayload); + await _messageBroker.SendToUserAsync(userId, errorResponse);.env.example (1)
19-22: SERVER_ID 자동 생성 주석 추가 제안분산 환경에서 중복 방지를 위해 비워두면 런타임 자동 생성됨을 명시하면 설정 실수를 줄일 수 있습니다.
# Distributed System Configuration DISTRIBUTED_MODE=false +# SERVER_ID가 비어 있으면 런타임에서 호스트명+GUID 등으로 자동 생성됩니다. SERVER_ID=ProjectVG.Api/ApiMiddlewareExtensions.cs (2)
27-30: WebSocket 옵션 적용 LGTM + 파이프라인 순서 점검 권장UseRouting → UseAuthentication/Authorization → UseEndpoints 순서를 권장합니다. 현재 순서(인증이 Routing 이전)는 정책 적용에 영향 줄 수 있습니다.
69-118: 환경 변수 파싱 Culture 고정 및 로깅 개선
- 지역 문화권에 따라 double 파싱 실패 가능성 존재 → InvariantCulture 사용 권장.
- Console.WriteLine 대신 ILogger 사용 권장.
- if (double.TryParse(keepAliveMinutes, out var minutes)) + if (double.TryParse(keepAliveMinutes, System.Globalization.NumberStyles.Float, System.Globalization.CultureInfo.InvariantCulture, out var minutes)) { if (minutes <= 0) { options.KeepAliveInterval = TimeSpan.Zero; // KeepAlive 비활성화 } else { options.KeepAliveInterval = TimeSpan.FromMinutes(minutes); } }추가(파일 상단 외부 변경):
using System.Globalization; 추가.ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (7)
201-240: async void 콜백 지양: Task.Run으로 비동기 작업 분리 + void 시그니처로 변경Redis 구독 콜백은 동기 델리게이트를 요구합니다. async void는 예외/수명 관리가 불명확합니다. 아래처럼 void + Task.Run 패턴을 권장합니다.
- private async void OnUserMessageReceived(RedisChannel channel, RedisValue message) - { - try - { - ... - var success = await _connectionManager.SendTextAsync(brokerMessage.TargetUserId, payloadText); - ... - } - catch (Exception ex) - { - ... - } - } + private void OnUserMessageReceived(RedisChannel channel, RedisValue message) + { + _ = Task.Run(async () => + { + try + { + ... + var success = await _connectionManager.SendTextAsync(brokerMessage.TargetUserId, payloadText); + ... + } + catch (Exception ex) + { + ... + } + }); + }
242-288: 서버 메시지 콜백도 async void 제거 권장동일한 이유로 void + Task.Run 패턴으로 전환해 주세요.
- private async void OnServerMessageReceived(RedisChannel channel, RedisValue message) + private void OnServerMessageReceived(RedisChannel channel, RedisValue message) { - try - { - ... - await SendLocalMessageAsJson(brokerMessage.TargetUserId, brokerMessage.Payload); - ... - } - catch (Exception ex) - { - ... - } + _ = Task.Run(async () => + { + try + { + ... + await SendLocalMessageAsJson(brokerMessage.TargetUserId, brokerMessage.Payload); + ... + } + catch (Exception ex) + { + ... + } + }); }
290-357: 브로드캐스트 콜백의 async void 제거 및 백그라운드 태스크로 처리다수 전송/집계를 유지하되, 예외 전파와 수명 관리를 위해 void + Task.Run 패턴으로 전환하세요.
- private async void OnBroadcastMessageReceived(RedisChannel channel, RedisValue message) + private void OnBroadcastMessageReceived(RedisChannel channel, RedisValue message) { - try - { - ... - await Task.WhenAll(broadcastTasks).ConfigureAwait(false); - ... - } - catch (Exception ex) - { - ... - } + _ = Task.Run(async () => + { + try + { + ... + await Task.WhenAll(broadcastTasks).ConfigureAwait(false); + ... + } + catch (Exception ex) + { + ... + } + }); }
80-81: 오타 수정: "분살브로커" → "분산브로커"로그 메시지 오타입니다.
- _logger.LogError(ex, "[분살브로커] {ErrorMessage}: ServerId={ServerId}", errorMessage, _serverId); + _logger.LogError(ex, "[분산브로커] {ErrorMessage}: ServerId={ServerId}", errorMessage, _serverId);
192-193: 오타 수정: "완뢬" → "완료"- _logger.LogDebug("사용자 채널 구독 해제 완뢬: {UserId}", userId); + _logger.LogDebug("사용자 채널 구독 해제 완료: {UserId}", userId);
341-345: 주석 오타 정정"대학" → "요약" 또는 "대략".
- // 최종 성공/실패 대학 요약만 로깅 + // 최종 성공/실패 요약만 로깅
72-76: 사용자 채널 구독/해제 미사용 – 제거 또는 호출 로직 추가 필요
SubscribeToUserChannelAsync/UnsubscribeFromUserChannelAsync가 코드 내에서 호출되지 않습니다. 해당 메서드를 실제 사용자 연결/해제 시점에 배선하거나 불필요하다면 제거하세요.ProjectVG.Application/Services/Chat/Processors/ChatResultProcessor.cs (1)
3-27: 미사용 의존성 제거 또는 실제 사용으로 변경IMessageBroker 주입 후 사용되지 않습니다. 제거하거나 후속 알림/브로드캐스트 등 실제 사용 지점을 추가하세요.
-using ProjectVG.Application.Services.MessageBroker; ... - private readonly IMessageBroker _messageBroker; ... - IMemoryClient memoryClient, - IMessageBroker messageBroker) + IMemoryClient memoryClient) { _logger = logger; _conversationService = conversationService; _memoryClient = memoryClient; - _messageBroker = messageBroker; }ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (5)
14-16: 키 프리픽스 명명 혼선 및 인덱스 키 추가 제안세션 키가 sessionId 기준인데 프리픽스가 "session:user:"로 되어 있어 혼동을 유발합니다. 또한 이후 제안하는 인덱스 기반 조회/카운트에 대비해 전역/사용자 인덱스 키를 정의해 두는 것을 권장합니다.
아래처럼 교체/추가를 고려해 주세요:
- private const string SESSION_KEY_PREFIX = "session:user:"; - private static readonly TimeSpan SESSION_TTL = TimeSpan.FromMinutes(30); + private const string SESSION_KEY_PREFIX = "session:"; + private const string SESSIONS_INDEX_KEY = "session:index:all"; + private const string USER_SESSIONS_INDEX_PREFIX = "session:index:user:"; + private static readonly TimeSpan SESSION_TTL = TimeSpan.FromMinutes(30);주의: 운영 중 키 프리픽스 변경은 마이그레이션이 필요합니다. 레거시 키를 병행 지원하거나 백필 스크립트가 필요할 수 있습니다.
24-46: 역직렬화 시 불필요한 문자열 할당 — Span 사용으로 할당 최소화RedisValue → string 변환을 거치지 말고 바로 Span 기반 역직렬화를 사용하면 할당을 줄일 수 있습니다.
- var sessionInfo = JsonSerializer.Deserialize<SessionInfo>(value!); + var sessionInfo = JsonSerializer.Deserialize<SessionInfo>(value.Span);
150-165: 로그 레벨 완화 제안Exists 확인 로그는 정보 레벨로 남기기엔 소음이 큽니다. Debug로 낮추는 것을 권장합니다.
- _logger.LogInformation("[RedisSessionStorage] 세션 존재 확인: SessionId={SessionId}, RedisKey={RedisKey}, Exists={Exists}", + _logger.LogDebug("[RedisSessionStorage] 세션 존재 확인: SessionId={SessionId}, RedisKey={RedisKey}, Exists={Exists}", sessionId, key, exists);
202-224: Heartbeat 왕복 최소화 가능 — Lua/KeyExpire로 최적화 검토현재 Get→Update로 2회 RTT와 전체 JSON 재쓰기입니다. LastActivity 갱신과 TTL 연장을 함께 처리하는 Lua 스크립트(원자적)나, LastActivity가 로그용이라면 KeyExpire만으로 간소화하는 옵션을 검토해 주세요. 분산 트래픽(하트비트 빈번)에서 의미 있는 절감이 됩니다.
24-24: 인덱스 도입에 따른 보조 코드(파일 외부 라인) 제안위 제안들을 적용하려면 다음 보조 요소가 필요합니다.
- 상수: SESSIONS_INDEX_KEY, USER_SESSIONS_INDEX_PREFIX (라인 14-16 코멘트 참조)
- 선택: 사용자 인덱스 키 헬퍼
추가 헬퍼 예시:
private static string GetUserIndexKey(string userId) => USER_SESSIONS_INDEX_PREFIX + userId;인덱스 초기화/백필이 필요하면 일회성 마이그레이션 스크립트로 기존 세션 키를 스캔해 Set에 sessionId를 적재해야 합니다. 필요 시 스크립트 제공 가능합니다.
Also applies to: 48-48, 79-79, 127-127, 185-185
ProjectVG.Application/Services/Session/RedisSessionManager.cs (1)
129-137: 성능 저하 가능성이 있는 전체 스캔 사용활성 사용자 ID를 모으기 위해
GetAllAsync()로 전체 세션을 로드한 뒤 LINQ로 필터링하고 있습니다. Redis 측에서 이미 사용자 ID 목록을 직전 쿼리로 제공할 수 있도록GetActiveUserIdsAsync혹은 유사 API가 있다면, 그 쪽을 호출하도록 리팩터링하는 편이 좋겠습니다. 현재 구현은 사용자 수가 늘어날수록 세션 전체를 스캔하게 되어 Redis 부하와 응답 지연이 커질 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.env.example(2 hunks)ProjectVG.Api/ApiMiddlewareExtensions.cs(3 hunks)ProjectVG.Api/Middleware/WebSocketMiddleware.cs(5 hunks)ProjectVG.Api/appsettings.json(1 hunks)ProjectVG.Application/ApplicationServiceCollectionExtensions.cs(1 hunks)ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs(1 hunks)ProjectVG.Application/Services/Chat/ChatService.cs(1 hunks)ProjectVG.Application/Services/Chat/Handlers/ChatFailureHandler.cs(1 hunks)ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs(2 hunks)ProjectVG.Application/Services/Chat/Processors/ChatResultProcessor.cs(2 hunks)ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs(3 hunks)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs(1 hunks)ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs(1 hunks)ProjectVG.Application/Services/MessageBroker/MessageBrokerInitializationService.cs(1 hunks)ProjectVG.Application/Services/Session/ConnectionRegistry.cs(0 hunks)ProjectVG.Application/Services/Session/IConnectionRegistry.cs(0 hunks)ProjectVG.Application/Services/Session/ISessionManager.cs(1 hunks)ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs(1 hunks)ProjectVG.Application/Services/Session/RedisSessionManager.cs(1 hunks)ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs(1 hunks)ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs(0 hunks)ProjectVG.Application/Services/WebSocket/WebSocketManager.cs(0 hunks)ProjectVG.Common/Constants/ErrorCodes.cs(2 hunks)ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs(3 hunks)ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs(1 hunks)ProjectVG.Tests/Application/Services/Chat/ChatServiceSimpleTests.cs(3 hunks)ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs(10 hunks)ProjectVG.Tests/Services/Chat/Validator/ChatRequestValidatorTests.cs(4 hunks)
💤 Files with no reviewable changes (4)
- ProjectVG.Application/Services/Session/IConnectionRegistry.cs
- ProjectVG.Application/Services/WebSocket/IWebSocketManager.cs
- ProjectVG.Application/Services/Session/ConnectionRegistry.cs
- ProjectVG.Application/Services/WebSocket/WebSocketManager.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- ProjectVG.Application/Services/Chat/ChatService.cs
- ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs
- ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs
🧰 Additional context used
🧬 Code graph analysis (16)
ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs (1)
ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs (5)
RegisterConnection(21-35)IClientConnection(155-171)UnregisterConnection(37-55)HasLocalConnection(57-73)GetLocalConnectionCount(123-137)
ProjectVG.Application/Services/Session/RedisSessionManager.cs (2)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (4)
Task(34-56)Task(97-147)Task(152-276)Guid(61-77)ProjectVG.Common/Models/Session/SessionInfo.cs (1)
SessionInfo(3-9)
ProjectVG.Application/Services/Session/ISessionManager.cs (3)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (1)
Guid(61-77)ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs (1)
IEnumerable(56-56)ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs (1)
IEnumerable(139-153)
ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs (3)
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (2)
RedisSessionStorage(8-230)RedisSessionStorage(17-22)ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (2)
RedisServerRegistrationService(9-335)RedisServerRegistrationService(23-31)ProjectVG.Infrastructure/Services/Server/ServerLifecycleService.cs (2)
ServerLifecycleService(13-112)ServerLifecycleService(23-29)
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs (2)
ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (2)
ChatSuccessHandler(9-128)ChatSuccessHandler(15-23)ProjectVG.Application/Models/WebSocket/WebSocketMessage.cs (2)
WebSocketMessage(16-16)WebSocketMessage(18-23)
ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (1)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (4)
Task(34-56)Task(97-147)Task(152-276)Guid(61-77)
ProjectVG.Tests/Services/Chat/Validator/ChatRequestValidatorTests.cs (1)
ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (2)
ChatRequestValidator(10-124)ChatRequestValidator(21-33)
ProjectVG.Application/Services/MessageBroker/MessageBrokerInitializationService.cs (1)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (7)
Task(85-122)Task(124-137)Task(139-155)Task(160-177)Task(182-199)Task(359-407)Task(409-447)
ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs (3)
ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs (6)
IClientConnection(63-63)RegisterConnection(15-15)UnregisterConnection(21-21)HasLocalConnection(28-28)GetLocalConnectionCount(50-50)IEnumerable(56-56)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (7)
Task(85-122)Task(124-137)Task(139-155)Task(160-177)Task(182-199)Task(359-407)Task(409-447)ProjectVG.Api/Middleware/WebSocketMiddleware.cs (3)
Task(34-56)Task(97-147)Task(152-276)
ProjectVG.Application/Services/Chat/Handlers/ChatFailureHandler.cs (3)
ProjectVG.Application/Services/Chat/ChatService.cs (3)
Task(65-80)Task(86-103)Task(108-129)ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (2)
Task(25-89)Task(94-127)ProjectVG.Application/Models/Chat/ChatProcessContext.cs (2)
ChatProcessContext(23-31)ChatProcessContext(33-49)
ProjectVG.Api/Middleware/WebSocketMiddleware.cs (7)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (7)
Task(85-122)Task(124-137)Task(139-155)Task(160-177)Task(182-199)Task(359-407)Task(409-447)ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (2)
Task(35-69)Task(75-123)ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs (3)
RegisterConnection(15-15)HasLocalConnection(28-28)UnregisterConnection(21-21)ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs (3)
RegisterConnection(21-35)HasLocalConnection(57-73)UnregisterConnection(37-55)ProjectVG.Infrastructure/Realtime/WebSocketConnection/WebSocketClientConnection.cs (2)
WebSocketClientConnection(11-84)WebSocketClientConnection(20-25)ProjectVG.Domain/Services/Server/IServerRegistrationService.cs (1)
GetServerId(25-25)ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (1)
GetServerId(125-128)
ProjectVG.Tests/Application/Services/Chat/ChatServiceSimpleTests.cs (2)
ProjectVG.Application/Services/Chat/Handlers/ChatFailureHandler.cs (2)
ChatFailureHandler(7-30)ChatFailureHandler(12-18)ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs (2)
ChatRequestValidator(10-124)ChatRequestValidator(21-33)
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (8)
ProjectVG.Infrastructure/Services/Server/RedisServerRegistrationService.cs (1)
GetServerId(125-128)ProjectVG.Application/Services/Server/IServerRegistrationService.cs (1)
GetServerId(25-25)ProjectVG.Api/Middleware/WebSocketMiddleware.cs (3)
Task(34-56)Task(97-147)Task(152-276)ProjectVG.Application/Services/MessageBroker/MessageBrokerInitializationService.cs (2)
Task(22-41)Task(43-47)ProjectVG.Application/Services/MessageBroker/LocalMessageBroker.cs (3)
Task(26-59)Task(61-83)Task(85-90)ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs (1)
HasLocalConnection(28-28)ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs (1)
HasLocalConnection(57-73)ProjectVG.Application/Models/MessageBroker/BrokerMessage.cs (6)
BrokerMessage(5-101)BrokerMessage(16-42)BrokerMessage(44-57)BrokerMessage(59-71)BrokerMessage(90-100)ToJson(85-88)
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (6)
ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs (2)
IServiceCollection(25-36)AddAuthServices(128-164)ProjectVG.Application/Services/Chat/CostTracking/CostTrackingDecoratorFactory.cs (1)
IServiceCollection(7-28)ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (2)
DistributedMessageBroker(17-463)DistributedMessageBroker(32-59)ProjectVG.Application/Services/Session/RedisSessionManager.cs (2)
RedisSessionManager(10-145)RedisSessionManager(15-19)ProjectVG.Application/Services/MessageBroker/MessageBrokerInitializationService.cs (2)
MessageBrokerInitializationService(9-48)MessageBrokerInitializationService(14-20)ProjectVG.Application/Services/Session/WebSocketConnectionManager.cs (2)
WebSocketConnectionManager(10-172)WebSocketConnectionManager(15-19)
ProjectVG.Api/ApiMiddlewareExtensions.cs (1)
ProjectVG.Tests/Api/Configuration/ConfigurationExtensionsTests.cs (1)
IConfiguration(435-440)
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (1)
ProjectVG.Common/Models/Session/SessionInfo.cs (1)
SessionInfo(3-9)
🪛 GitHub Check: Build & Test
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs
[warning] 454-454:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 453-453:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 187-187:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 165-165:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 146-146:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 129-129:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 112-112:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 70-70:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
[warning] 67-67:
'RedisChannel.implicit operator RedisChannel(string)' is obsolete: 'It is preferable to explicitly specify a PatternMode, or use the Literal/Pattern methods'
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs
[warning] 97-97:
Possible null reference argument for parameter 'sessionStorage' in 'RedisSessionManager.RedisSessionManager(ISessionStorage sessionStorage, ILogger logger)'.
🪛 dotenv-linter (3.3.0)
.env.example
[warning] 7-7: [UnorderedKey] The DEBUG_MODE key should go before the ENVIRONMENT key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The LOG_FORMAT key should go before the LOG_LEVEL key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The DATA_PATH key should go before the DEBUG_MODE key
(UnorderedKey)
[warning] 13-13: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🔇 Additional comments (19)
ProjectVG.Application/Services/Chat/Handlers/ChatFailureHandler.cs (1)
10-18: IMessageBroker 전환 LGTM의존성 교체 및 생성자 주입 변경이 일관성 있게 반영되었습니다.
ProjectVG.Api/appsettings.json (3)
16-19: ServerId 하드코딩 제거 필요 (이전 코멘트와 동일)다중 인스턴스 충돌/세션 오염 위험. 환경변수 주입 또는 런타임 생성으로 대체하세요. CleanupInterval 단축/단위 정합성도 함께 검토 권장.
35-37: 평문 DB 비밀번호 커밋됨 — 즉시 제거/회수 필요 (이전 코멘트와 동일)환경변수/시크릿으로 주입하고 Git 이력에서 정리하세요.
37-37: Redis 인증/TLS 미설정 (이전 코멘트와 동일)프로덕션에서는 인증/암호화 필수(rediss:// 또는 ssl=true).
ProjectVG.Api/ApiMiddlewareExtensions.cs (1)
14-14: 구성 주입으로 WebSocket 옵션 구성 — 좋습니다DI에서 IConfiguration을 가져와 옵션화한 접근이 적절합니다.
ProjectVG.Application/Services/MessageBroker/DistributedMessageBroker.cs (2)
67-71: RedisChannel 암시적 변환 경고 제거: Literal 사용으로 교체하세요StackExchange.Redis 권고에 따라 채널 인자에 RedisChannel.Literal(...)을 사용해야 합니다. 아래 패치로 모든 경고 제거 가능합니다.
- _subscriber.Subscribe(serverChannel, OnServerMessageReceived); + _subscriber.Subscribe(RedisChannel.Literal(serverChannel), OnServerMessageReceived); - _subscriber.Subscribe(BROADCAST_CHANNEL, OnBroadcastMessageReceived); + _subscriber.Subscribe(RedisChannel.Literal(BROADCAST_CHANNEL), OnBroadcastMessageReceived); - await _subscriber.PublishAsync(serverChannel, brokerMessage.ToJson()); + await _subscriber.PublishAsync(RedisChannel.Literal(serverChannel), brokerMessage.ToJson()); - await _subscriber.PublishAsync(BROADCAST_CHANNEL, brokerMessage.ToJson()); + await _subscriber.PublishAsync(RedisChannel.Literal(BROADCAST_CHANNEL), brokerMessage.ToJson()); - await _subscriber.SubscribeAsync(userChannel, OnUserMessageReceived); + await _subscriber.SubscribeAsync(RedisChannel.Literal(userChannel), OnUserMessageReceived); - await _subscriber.UnsubscribeAsync(userChannel); + await _subscriber.UnsubscribeAsync(RedisChannel.Literal(userChannel)); - _subscriber?.Unsubscribe($"{SERVER_CHANNEL_PREFIX}:{_serverId}"); - _subscriber?.Unsubscribe(BROADCAST_CHANNEL); + _subscriber?.Unsubscribe(RedisChannel.Literal($"{SERVER_CHANNEL_PREFIX}:{_serverId}")); + _subscriber?.Unsubscribe(RedisChannel.Literal(BROADCAST_CHANNEL));Also applies to: 112-115, 129-131, 146-148, 165-171, 187-193, 453-456
254-265: 원본 JSON 페이로드 직접 전달 적용, 일관성 확보브로커 저장 JSON을 그대로 전송하도록 바뀐 점 좋습니다. 로컬/원격 경로 JSON 스키마가 일치합니다.
ProjectVG.Application/ApplicationServiceCollectionExtensions.cs (2)
77-104: DI 순환/라이프사이클 점검 OK
- IMessageBroker/IWebSocketConnectionManager를 Singleton으로 등록한 점 적절합니다.
- HostedService로 초기화 강제도 합리적입니다.
브로커 팩토리 해석 시 필요한 IWebSocketConnectionManager/IServerRegistrationService가 모두 등록되어 있는지(인프라 확장 메서드 포함) 로컬에서 한 번 부팅 확인 부탁드립니다.
26-34: AddApplicationServices 호출부에 IConfiguration 전달 확인됨
ProgramVG.Api/Program.cs:41 –builder.Services.AddApplicationServices(builder.Configuration)로 올바르게 업데이트되었습니다.ProjectVG.Common/Constants/ErrorCodes.cs (1)
87-104: 새 오류 코드 추가 적절분산/Redis 경로에 필요한 코드와 메시지 매핑이 잘 정리되었습니다.
Also applies to: 192-209
ProjectVG.Application/Services/MessageBroker/MessageBrokerInitializationService.cs (1)
22-41: 초기화 시퀀스 분리 좋습니다호스트 시작 시 브로커 초기화를 강제하는 방식 적절합니다.
ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs (3)
48-77: server.Keys + First()는 클러스터에서 키 누락/병목 — 인덱스(Set) 기반 조회로 교체 필요클러스터/샤드 환경에서
_redis.GetEndPoints().First()는 일부 샤드만 스캔하여 누락이 발생합니다. 또한 Keys 스캔+per-key GET은 고비용입니다. 전역 인덱스 Set(SESSIONS_INDEX_KEY)에 sessionId를 유지하고, batched MGET으로 조회하세요. (이전 코멘트 중복)- var server = _redis.GetServer(_redis.GetEndPoints().First()); - var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*"); - var sessions = new List<SessionInfo>(); - - foreach (var key in keys) - { - var value = await _database.StringGetAsync(key); - if (value.HasValue) - { - var session = JsonSerializer.Deserialize<SessionInfo>(value!); - if (session != null) - { - sessions.Add(session); - } - } - } + var ids = await _database.SetMembersAsync(SESSIONS_INDEX_KEY); + if (ids.Length == 0) + return Enumerable.Empty<SessionInfo>(); + + var keys = new RedisKey[ids.Length]; + for (int i = 0; i < ids.Length; i++) + keys[i] = (RedisKey)(SESSION_KEY_PREFIX + (string)ids[i]); + + var values = await _database.StringGetAsync(keys); + var sessions = new List<SessionInfo>(values.Length); + foreach (var v in values) + { + if (!v.HasValue) continue; + var s = JsonSerializer.Deserialize<SessionInfo>(v.Span); + if (s != null) sessions.Add(s); + }단기완화(인덱스 도입 전 임시): 모든 endpoint에 대해 Keys를 합산하세요:
_redis.GetEndPoints()순회 +GetServer(ep).Keys(database: _database.Database, pattern: ..., pageSize: 1000).
167-183: 활성 세션 수 계산에 server.Keys 사용 — 클러스터/성능 문제. 인덱스(SetLength)로 변경 필요이전 리뷰에서 지적된 동일 이슈입니다. Keys 스캔은 누락/병목/관리 권한 의존 문제가 있습니다. 인덱스 Set의 길이를 사용하세요.
- public Task<int> GetActiveSessionCountAsync() + public async Task<int> GetActiveSessionCountAsync() { try { - var server = _redis.GetServer(_redis.GetEndPoints().First()); - var keys = server.Keys(pattern: SESSION_KEY_PREFIX + "*"); - var count = keys.Count(); - - _logger.LogDebug("활성 세션 수: {Count}", count); - return Task.FromResult(count); + var count = await _database.SetLengthAsync(SESSIONS_INDEX_KEY); + _logger.LogDebug("활성 세션 수: {Count}", count); + return (int)count; } catch (Exception ex) { _logger.LogError(ex, "활성 세션 수 조회 실패"); - return Task.FromResult(0); + return 0; } }단기완화: 인덱스가 아직 없다면 모든 endpoint에 대해 Keys를 합산하세요.
79-105: StringSetAsync 실패 시 예외 처리 및 원자성 보장 고려
setResult가 false인 경우 예외를 던져 저장 실패를 호출자에게 알리도록 수정- 필요 시 ITransaction 또는 Lua 스크립트를 사용해 저장과 후속 처리를 원자적으로 묶을 것
Likely an incorrect or invalid review comment.
ProjectVG.Tests/Application/Services/Chat/ChatServiceSimpleTests.cs (1)
198-243: 새 의존성(mock) 정렬이 매끄럽습니다ISessionManager와 IMessageBroker 목 객체를 일관되게 주입해 새 아키텍처 변경과 테스트 셋업이 잘 맞춰졌습니다.
ProjectVG.Tests/Services/Chat/Handlers/ChatSuccessHandlerTests.cs (1)
54-195: 메시지 브로커 기반 검증이 깔끔하게 조정되었습니다SendToUserAsync를 대상으로 한 Verify와 Callback 시그니처가 IMessageBroker 계약과 정확히 일치해 회귀 걱정이 없습니다.
ProjectVG.Application/Services/Session/IWebSocketConnectionManager.cs (1)
30-64: 로컬 WebSocket 관리 계약 정의 좋아요텍스트/바이너리 전송과 로컬 상태 조회가 명확히 분리돼 구현체 설계가 수월해졌습니다.
ProjectVG.Application/Services/Chat/Handlers/ChatSuccessHandler.cs (1)
12-71: 메시지 브로커 전환 로깅/전송 흐름이 탄탄합니다분산 여부 로그와 SendToUserAsync 호출이 잘 연결돼 운용 시 가시성과 안정성이 기대됩니다.
ProjectVG.Application/Services/Session/ISessionManager.cs (1)
8-46: 세션 라이프사이클 계약이 명료하게 정리됐습니다생성·하트비트·삭제·조회 기능이 한 인터페이스에 모여 Redis/분산 구현 교체가 수월하겠습니다.
| ENVIRONMENT=development | ||
| DEBUG_MODE=true | ||
| LOG_LEVEL=INFO | ||
| LOG_FORMAT=json | ||
| DATA_PATH=../data |
There was a problem hiding this comment.
ENVIRONMENT vs ASPNETCORE_ENVIRONMENT 값 충돌
두 변수가 서로 다른 값을 갖고 있어 실제 실행 환경이 오인될 수 있습니다. 하나로 통일하세요.
-ASPNETCORE_ENVIRONMENT=Production
+ASPNETCORE_ENVIRONMENT=Development(또는 ENVIRONMENT 키를 제거/주석 처리하고 ASPNETCORE_ENVIRONMENT만 사용)
Also applies to: 50-50
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 7-7: [UnorderedKey] The DEBUG_MODE key should go before the ENVIRONMENT key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The LOG_FORMAT key should go before the LOG_LEVEL key
(UnorderedKey)
[warning] 10-10: [UnorderedKey] The DATA_PATH key should go before the DEBUG_MODE key
(UnorderedKey)
🤖 Prompt for AI Agents
In .env.example around lines 6 to 10 (and similarly at lines ~50), there is a
potential conflict between ENVIRONMENT and ASPNETCORE_ENVIRONMENT having
different values; pick one environment variable to be authoritative (preferably
ASPNETCORE_ENVIRONMENT for ASP.NET apps) and either remove or comment out the
duplicate ENVIRONMENT entry, or set both to the same value to avoid mismatched
runtime behavior; update the file so only the chosen key remains (or both match)
and add a brief comment explaining which variable the app reads.
| DATA_PATH=../data | ||
|
|
||
| # Database Configuration | ||
| DB_CONNECTION_STRING=Server=host.docker.internal,1433;Database=ProjectVG;User Id=sa;Password=YOUR_DB_PASSWORD;TrustServerCertificate=true;MultipleActiveResultSets=true |
There was a problem hiding this comment.
DB 연결 문자열은 따옴표로 감싸기
특수문자 포함 값은 인용이 안전합니다. dotenv-linter 경고도 해소됩니다.
-DB_CONNECTION_STRING=Server=host.docker.internal,1433;Database=ProjectVG;User Id=sa;Password=YOUR_DB_PASSWORD;TrustServerCertificate=true;MultipleActiveResultSets=true
+DB_CONNECTION_STRING="Server=host.docker.internal,1433;Database=ProjectVG;User Id=sa;Password=YOUR_DB_PASSWORD;TrustServerCertificate=true;MultipleActiveResultSets=true"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| DB_CONNECTION_STRING=Server=host.docker.internal,1433;Database=ProjectVG;User Id=sa;Password=YOUR_DB_PASSWORD;TrustServerCertificate=true;MultipleActiveResultSets=true | |
| DB_CONNECTION_STRING="Server=host.docker.internal,1433;Database=ProjectVG;User Id=sa;Password=YOUR_DB_PASSWORD;TrustServerCertificate=true;MultipleActiveResultSets=true" |
🧰 Tools
🪛 dotenv-linter (3.3.0)
[warning] 13-13: [ValueWithoutQuotes] This value needs to be surrounded in quotes
(ValueWithoutQuotes)
🤖 Prompt for AI Agents
.env.example around line 13: the DB_CONNECTION_STRING value contains special
characters and should be wrapped in quotes to be safe and satisfy dotenv-linter;
update the line to enclose the entire connection string in double quotes (e.g.
DB_CONNECTION_STRING="Server=...;Password=...;...") so parsers and linters treat
it as a single quoted value.
| // 새 아키텍처: 세션과 로컬 연결 분리해서 정리 | ||
| if (Guid.TryParse(userId, out var userGuid)) | ||
| { | ||
| // 1. 세션 관리자에서 세션 삭제 (Redis에서 제거) | ||
| await _sessionManager.DeleteSessionAsync(userGuid); | ||
| _logger.LogDebug("세션 관리자에서 세션 삭제 완료: {UserId}", userId); | ||
| } | ||
|
|
||
| // 2. 분산 시스템: 사용자-서버 매핑 제거 (Redis) | ||
| if (_serverRegistrationService != null) | ||
| { | ||
| try | ||
| { | ||
| await _serverRegistrationService.RemoveUserServerAsync(userId); | ||
| _logger.LogDebug("사용자-서버 매핑 제거 완료: {UserId}", userId); | ||
| } | ||
| catch (Exception mapEx) | ||
| { | ||
| _logger.LogWarning(mapEx, "사용자-서버 매핑 제거 실패: {UserId}", userId); | ||
| } | ||
| } | ||
|
|
||
| // 3. 로컬 WebSocket 연결 해제 | ||
| _connectionManager.UnregisterConnection(userId); | ||
| _logger.LogDebug("로컬 WebSocket 연결 해제 완료: {UserId}", userId); | ||
|
|
There was a problem hiding this comment.
재연결 시 신규 WebSocket 세션까지 정리되는 치명적 버그
동일 사용자로 새로운 WebSocket이 붙으면, 기존 루프가 finally 블록을 실행하면서 _sessionManager.DeleteSessionAsync와 _connectionManager.UnregisterConnection을 다시 호출합니다. 현재 코드는 사용자 ID만 기준으로 공유 상태를 정리하기 때문에, 새로 등록된 연결까지 통째로 지워지면서 분산 브로커 매핑과 로컬 연결이 동시에 사라집니다.
아래처럼 현재 소켓이 여전히 매니저에 등록된 소켓과 동일한 경우에만 공유 상태를 정리하도록 가드가 필요합니다.
try {
- // 새 아키텍처: 세션과 로컬 연결 분리해서 정리
- if (Guid.TryParse(userId, out var userGuid))
- {
- // 1. 세션 관리자에서 세션 삭제 (Redis에서 제거)
- await _sessionManager.DeleteSessionAsync(userGuid);
- _logger.LogDebug("세션 관리자에서 세션 삭제 완료: {UserId}", userId);
- }
-
- // 2. 분산 시스템: 사용자-서버 매핑 제거 (Redis)
- if (_serverRegistrationService != null)
- {
- try
- {
- await _serverRegistrationService.RemoveUserServerAsync(userId);
- _logger.LogDebug("사용자-서버 매핑 제거 완료: {UserId}", userId);
- }
- catch (Exception mapEx)
- {
- _logger.LogWarning(mapEx, "사용자-서버 매핑 제거 실패: {UserId}", userId);
- }
- }
-
- // 3. 로컬 WebSocket 연결 해제
- _connectionManager.UnregisterConnection(userId);
- _logger.LogDebug("로컬 WebSocket 연결 해제 완료: {UserId}", userId);
+ var currentConnection = _connectionManager.GetConnection(userId);
+ var ownsRegistration = currentConnection == null;
+
+ if (currentConnection is WebSocketClientConnection registeredConnection)
+ {
+ ownsRegistration = ReferenceEquals(registeredConnection.WebSocket, socket);
+ }
+ else if (currentConnection != null)
+ {
+ ownsRegistration = false;
+ }
+
+ if (!ownsRegistration)
+ {
+ _logger.LogDebug("다른 WebSocket 연결이 이미 등록되어 있어 공유 상태 정리를 건너뜁니다: {UserId}", userId);
+ }
+ else
+ {
+ if (Guid.TryParse(userId, out var userGuid))
+ {
+ // 1. 세션 관리자에서 세션 삭제 (Redis에서 제거)
+ await _sessionManager.DeleteSessionAsync(userGuid);
+ _logger.LogDebug("세션 관리자에서 세션 삭제 완료: {UserId}", userId);
+ }
+
+ // 2. 분산 시스템: 사용자-서버 매핑 제거 (Redis)
+ if (_serverRegistrationService != null)
+ {
+ try
+ {
+ await _serverRegistrationService.RemoveUserServerAsync(userId);
+ _logger.LogDebug("사용자-서버 매핑 제거 완료: {UserId}", userId);
+ }
+ catch (Exception mapEx)
+ {
+ _logger.LogWarning(mapEx, "사용자-서버 매핑 제거 실패: {UserId}", userId);
+ }
+ }
+
+ // 3. 로컬 WebSocket 연결 해제
+ _connectionManager.UnregisterConnection(userId);
+ _logger.LogDebug("로컬 WebSocket 연결 해제 완료: {UserId}", userId);
+ }
// 4. WebSocket 소켓 정리
if (socket.State == WebSocketState.Open || socket.State == WebSocketState.CloseReceived) {
await socket.CloseAsync(
WebSocketCloseStatus.NormalClosure,
"Connection closed",
CancellationToken.None);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 새 아키텍처: 세션과 로컬 연결 분리해서 정리 | |
| if (Guid.TryParse(userId, out var userGuid)) | |
| { | |
| // 1. 세션 관리자에서 세션 삭제 (Redis에서 제거) | |
| await _sessionManager.DeleteSessionAsync(userGuid); | |
| _logger.LogDebug("세션 관리자에서 세션 삭제 완료: {UserId}", userId); | |
| } | |
| // 2. 분산 시스템: 사용자-서버 매핑 제거 (Redis) | |
| if (_serverRegistrationService != null) | |
| { | |
| try | |
| { | |
| await _serverRegistrationService.RemoveUserServerAsync(userId); | |
| _logger.LogDebug("사용자-서버 매핑 제거 완료: {UserId}", userId); | |
| } | |
| catch (Exception mapEx) | |
| { | |
| _logger.LogWarning(mapEx, "사용자-서버 매핑 제거 실패: {UserId}", userId); | |
| } | |
| } | |
| // 3. 로컬 WebSocket 연결 해제 | |
| _connectionManager.UnregisterConnection(userId); | |
| _logger.LogDebug("로컬 WebSocket 연결 해제 완료: {UserId}", userId); | |
| try | |
| { | |
| var currentConnection = _connectionManager.GetConnection(userId); | |
| var ownsRegistration = currentConnection == null; | |
| if (currentConnection is WebSocketClientConnection registeredConnection) | |
| { | |
| ownsRegistration = ReferenceEquals(registeredConnection.WebSocket, socket); | |
| } | |
| else if (currentConnection != null) | |
| { | |
| ownsRegistration = false; | |
| } | |
| if (!ownsRegistration) | |
| { | |
| _logger.LogDebug( | |
| "다른 WebSocket 연결이 이미 등록되어 있어 공유 상태 정리를 건너뜁니다: {UserId}", | |
| userId); | |
| } | |
| else | |
| { | |
| if (Guid.TryParse(userId, out var userGuid)) | |
| { | |
| // 1. 세션 관리자에서 세션 삭제 (Redis에서 제거) | |
| await _sessionManager.DeleteSessionAsync(userGuid); | |
| _logger.LogDebug( | |
| "세션 관리자에서 세션 삭제 완료: {UserId}", | |
| userId); | |
| } | |
| // 2. 분산 시스템: 사용자-서버 매핑 제거 (Redis) | |
| if (_serverRegistrationService != null) | |
| { | |
| try | |
| { | |
| await _serverRegistrationService.RemoveUserServerAsync(userId); | |
| _logger.LogDebug( | |
| "사용자-서버 매핑 제거 완료: {UserId}", | |
| userId); | |
| } | |
| catch (Exception mapEx) | |
| { | |
| _logger.LogWarning( | |
| mapEx, | |
| "사용자-서버 매핑 제거 실패: {UserId}", | |
| userId); | |
| } | |
| } | |
| // 3. 로컬 WebSocket 연결 해제 | |
| _connectionManager.UnregisterConnection(userId); | |
| _logger.LogDebug( | |
| "로컬 WebSocket 연결 해제 완료: {UserId}", | |
| userId); | |
| } | |
| // 4. WebSocket 소켓 정리 | |
| if (socket.State == WebSocketState.Open | |
| || socket.State == WebSocketState.CloseReceived) | |
| { | |
| await socket.CloseAsync( | |
| WebSocketCloseStatus.NormalClosure, | |
| "Connection closed", | |
| CancellationToken.None); | |
| } | |
| } |
🤖 Prompt for AI Agents
ProjectVG.Api/Middleware/WebSocketMiddleware.cs around lines 234 to 259: the
cleanup in finally indiscriminately deletes shared session and unregisters
connections by userId, which can remove a newly-registered WebSocket for the
same user; change the logic to first retrieve the currently-registered
connection for this user from _connectionManager and compare it to the
connection instance (or its connection Id) being closed, and only if they are
the same proceed to call _sessionManager.DeleteSessionAsync,
_serverRegistrationService.RemoveUserServerAsync and
_connectionManager.UnregisterConnection; keep null checks and exception handling
around the distributed removal, and ensure you do not delete session or
unregister if the registered connection differs or is already null.
| services.AddSingleton<ISessionManager>(serviceProvider => | ||
| { | ||
| var sessionStorage = serviceProvider.GetService<ISessionStorage>(); | ||
| var logger = serviceProvider.GetRequiredService<ILogger<RedisSessionManager>>(); | ||
| return new RedisSessionManager(sessionStorage, logger); | ||
| }); |
There was a problem hiding this comment.
Null 주입 위험: GetRequiredService 사용으로 조기 검증
ISessionStorage 해석 실패 시 null 주입됩니다. 시작 시점에 즉시 실패하도록 GetRequiredService로 변경하세요.
- var sessionStorage = serviceProvider.GetService<ISessionStorage>();
+ var sessionStorage = serviceProvider.GetRequiredService<ISessionStorage>();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| services.AddSingleton<ISessionManager>(serviceProvider => | |
| { | |
| var sessionStorage = serviceProvider.GetService<ISessionStorage>(); | |
| var logger = serviceProvider.GetRequiredService<ILogger<RedisSessionManager>>(); | |
| return new RedisSessionManager(sessionStorage, logger); | |
| }); | |
| services.AddSingleton<ISessionManager>(serviceProvider => | |
| { | |
| var sessionStorage = serviceProvider.GetRequiredService<ISessionStorage>(); | |
| var logger = serviceProvider.GetRequiredService<ILogger<RedisSessionManager>>(); | |
| return new RedisSessionManager(sessionStorage, logger); | |
| }); |
🧰 Tools
🪛 GitHub Check: Build & Test
[warning] 97-97:
Possible null reference argument for parameter 'sessionStorage' in 'RedisSessionManager.RedisSessionManager(ISessionStorage sessionStorage, ILogger logger)'.
🤖 Prompt for AI Agents
In ProjectVG.Application/ApplicationServiceCollectionExtensions.cs around lines
93 to 98, the factory uses serviceProvider.GetService<ISessionStorage>() which
can return null and cause a null-injection into RedisSessionManager; change that
call to serviceProvider.GetRequiredService<ISessionStorage>() so resolution
fails fast at startup if ISessionStorage is not registered, keeping the existing
ILogger resolution as-is.
| var activeSessionCount = await _sessionManager.GetActiveSessionCountAsync(); | ||
| var activeUserIds = await _sessionManager.GetActiveUserIdsAsync(); | ||
| _logger.LogInformation("[ChatRequestValidator] 현재 활성 세션: 총 {Count}개, UserIds=[{ActiveUserIds}]", | ||
| activeSessionCount, string.Join(", ", activeUserIds.Take(10))); // 너무 많은 로그 방지 | ||
|
|
||
| if (userSessions.Count == 0) { | ||
| _logger.LogWarning("유효하지 않은 사용자 세션: {UserId}", userId); | ||
| throw new ValidationException(ErrorCode.SESSION_EXPIRED, "세션이 만료되었습니다. 다시 로그인해 주세요."); | ||
| // 세션 관리자에서 세션 상태 확인 (Redis 기반) | ||
| bool isSessionActive = await _sessionManager.IsSessionActiveAsync(userId); | ||
|
|
||
| _logger.LogInformation("[ChatRequestValidator] 세션 상태 확인: UserId={UserId}, IsActive={IsActive}", | ||
| userId, isSessionActive); | ||
|
|
There was a problem hiding this comment.
로그용 세션 전체 조회로 인한 성능 저하 방지 필요
현재 ValidateUserSessionAsync가 매 호출마다 GetActiveSessionCountAsync()와 GetActiveUserIdsAsync()를 호출하여 Redis 전체 세션 목록을 긁어옵니다. RedisSessionManager.GetActiveUserIdsAsync()는 내부적으로 GetAllAsync()를 호출해 모든 세션을 로드하므로, 트래픽이 몰릴 때 Redis I/O와 직렬화 비용이 폭증해 채팅 요청 처리에 큰 병목이 됩니다. 단순 로깅 목적이라면 기본 흐름에서 이 호출들을 제거하거나 최소한 디버그 레벨일 때만 실행하도록 가드가 필요합니다.
다음과 같이 로그 가드를 추가해 주세요:
- var activeSessionCount = await _sessionManager.GetActiveSessionCountAsync();
- var activeUserIds = await _sessionManager.GetActiveUserIdsAsync();
- _logger.LogInformation("[ChatRequestValidator] 현재 활성 세션: 총 {Count}개, UserIds=[{ActiveUserIds}]",
- activeSessionCount, string.Join(", ", activeUserIds.Take(10))); // 너무 많은 로그 방지
+ if (_logger.IsEnabled(LogLevel.Debug))
+ {
+ var activeSessionCount = await _sessionManager.GetActiveSessionCountAsync();
+ var activeUserIds = await _sessionManager.GetActiveUserIdsAsync();
+ _logger.LogDebug("[ChatRequestValidator] 현재 활성 세션: 총 {Count}개, UserIds=[{ActiveUserIds}]",
+ activeSessionCount, string.Join(", ", activeUserIds.Take(10)));
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var activeSessionCount = await _sessionManager.GetActiveSessionCountAsync(); | |
| var activeUserIds = await _sessionManager.GetActiveUserIdsAsync(); | |
| _logger.LogInformation("[ChatRequestValidator] 현재 활성 세션: 총 {Count}개, UserIds=[{ActiveUserIds}]", | |
| activeSessionCount, string.Join(", ", activeUserIds.Take(10))); // 너무 많은 로그 방지 | |
| if (userSessions.Count == 0) { | |
| _logger.LogWarning("유효하지 않은 사용자 세션: {UserId}", userId); | |
| throw new ValidationException(ErrorCode.SESSION_EXPIRED, "세션이 만료되었습니다. 다시 로그인해 주세요."); | |
| // 세션 관리자에서 세션 상태 확인 (Redis 기반) | |
| bool isSessionActive = await _sessionManager.IsSessionActiveAsync(userId); | |
| _logger.LogInformation("[ChatRequestValidator] 세션 상태 확인: UserId={UserId}, IsActive={IsActive}", | |
| userId, isSessionActive); | |
| if (_logger.IsEnabled(LogLevel.Debug)) | |
| { | |
| var activeSessionCount = await _sessionManager.GetActiveSessionCountAsync(); | |
| var activeUserIds = await _sessionManager.GetActiveUserIdsAsync(); | |
| _logger.LogDebug("[ChatRequestValidator] 현재 활성 세션: 총 {Count}개, UserIds=[{ActiveUserIds}]", | |
| activeSessionCount, string.Join(", ", activeUserIds.Take(10))); | |
| } | |
| // 세션 관리자에서 세션 상태 확인 (Redis 기반) | |
| bool isSessionActive = await _sessionManager.IsSessionActiveAsync(userId); | |
| _logger.LogInformation("[ChatRequestValidator] 세션 상태 확인: UserId={UserId}, IsActive={IsActive}", | |
| userId, isSessionActive); |
🤖 Prompt for AI Agents
In ProjectVG.Application/Services/Chat/Validators/ChatRequestValidator.cs around
lines 80-90, the validator currently calls
_sessionManager.GetActiveSessionCountAsync() and
_sessionManager.GetActiveUserIdsAsync() on every invocation which causes heavy
Redis I/O and serialization; change this so those calls are only executed when
logging at a verbose level (e.g. wrap them in if
(_logger.IsEnabled(LogLevel.Debug)) { ... await ... _logger.LogDebug(...) }) or
remove them from the hot path entirely and only log a lightweight message (or
precomputed metric) otherwise; ensure you do not await or invoke GetAllAsync()
unless the logger guard passes.
| { | ||
| var distributedEnabled = configuration.GetValue<bool>("DistributedSystem:Enabled", false); | ||
|
|
||
| if (distributedEnabled) | ||
| { | ||
| // 분산 시스템이 활성화된 경우에만 등록 | ||
| services.AddScoped<ProjectVG.Domain.Services.Server.IServerRegistrationService, ProjectVG.Infrastructure.Services.Server.RedisServerRegistrationService>(); | ||
| services.AddHostedService<ProjectVG.Infrastructure.Services.Server.ServerLifecycleService>(); | ||
|
|
||
| Console.WriteLine("분산 시스템 모드 활성화"); | ||
| } | ||
| else | ||
| { | ||
| Console.WriteLine("단일 서버 모드"); | ||
| } | ||
| } |
There was a problem hiding this comment.
Redis 장애 시 분산 서비스 등록이 런타임을 깨뜨립니다
DistributedSystem:Enabled=true인 환경에서 Redis 연결이 실패하면 AddRedisServices가 IConnectionMultiplexer를 등록하지 않은 채 인메모리로 폴백합니다. 그런데 여기서는 그대로 RedisServerRegistrationService와 ServerLifecycleService를 DI에 추가해 버립니다. 애플리케이션이 부팅하면서 ServerLifecycleService가 IServerRegistrationService를 해석하려고 하면 IConnectionMultiplexer가 없어 InvalidOperationException이 발생하고 호스트가 바로 종료됩니다. Redis 장애를 감싸기 위한 폴백이 오히려 런타임을 깨뜨리는 상태입니다. Redis가 실제로 살아있는지(혹은 IConnectionMultiplexer가 등록돼 있는지) 확인한 뒤에만 분산 서비스를 추가하도록 수정해야 합니다.
private static void AddDistributedSystemServices(IServiceCollection services, IConfiguration configuration)
{
var distributedEnabled = configuration.GetValue<bool>("DistributedSystem:Enabled", false);
if (distributedEnabled)
{
+ if (!services.Any(sd => sd.ServiceType == typeof(IConnectionMultiplexer)))
+ {
+ Console.WriteLine("분산 시스템 모드가 설정되었으나 Redis 연결이 없어 단일 서버 모드로 전환됩니다.");
+ return;
+ }
+
// 분산 시스템이 활성화된 경우에만 등록
services.AddScoped<ProjectVG.Domain.Services.Server.IServerRegistrationService, ProjectVG.Infrastructure.Services.Server.RedisServerRegistrationService>();
services.AddHostedService<ProjectVG.Infrastructure.Services.Server.ServerLifecycleService>();🤖 Prompt for AI Agents
ProjectVG.Infrastructure/InfrastructureServiceCollectionExtensions.cs around
lines 221-236: currently you unconditionally register
RedisServerRegistrationService and ServerLifecycleService when
DistributedSystem:Enabled=true, which breaks startup if Redis failed and
IConnectionMultiplexer wasn't registered; modify the flow so that after calling
AddRedisServices (or after attempting to configure Redis) you verify Redis is
actually available—e.g., check that an IConnectionMultiplexer service was
successfully registered or perform a quick ping/IsConnected check via a
temporary service provider scope—and only then call
services.AddScoped<IServerRegistrationService, RedisServerRegistrationService>()
and services.AddHostedService<ServerLifecycleService>(); if the check fails, log
a warning and skip registering the distributed server services so the app keeps
running in single-server/fallback mode.
| public async Task<SessionInfo> UpdateAsync(SessionInfo session) | ||
| { | ||
| throw new NotImplementedException(); | ||
| try | ||
| { | ||
| var key = GetSessionKey(session.SessionId); | ||
| session.LastActivity = DateTime.UtcNow; | ||
|
|
||
| var json = JsonSerializer.Serialize(session); | ||
| await _database.StringSetAsync(key, json, SESSION_TTL); | ||
|
|
||
| _logger.LogDebug("세션 업데이트 성공 (하트비트): {SessionId}", session.SessionId); | ||
| return session; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "세션 업데이트 실패: {SessionId}", session.SessionId); | ||
| throw; | ||
| } | ||
| } |
There was a problem hiding this comment.
Update가 키 미존재 시 재생성 — When.Exists로 방지하고 실패 처리 필요
만료/삭제된 세션에 대해 Update가 새로운 세션을 “생성”시켜버릴 수 있습니다. 이는 세션 무결성을 해칩니다.
- await _database.StringSetAsync(key, json, SESSION_TTL);
+ var updated = await _database.StringSetAsync(key, json, SESSION_TTL, when: When.Exists);
+ if (!updated)
+ {
+ _logger.LogWarning("세션 업데이트 실패(키 없음): {SessionId}", session.SessionId);
+ throw new KeyNotFoundException($"세션을 찾을 수 없음: {session.SessionId}");
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public async Task<SessionInfo> UpdateAsync(SessionInfo session) | |
| { | |
| throw new NotImplementedException(); | |
| try | |
| { | |
| var key = GetSessionKey(session.SessionId); | |
| session.LastActivity = DateTime.UtcNow; | |
| var json = JsonSerializer.Serialize(session); | |
| await _database.StringSetAsync(key, json, SESSION_TTL); | |
| _logger.LogDebug("세션 업데이트 성공 (하트비트): {SessionId}", session.SessionId); | |
| return session; | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "세션 업데이트 실패: {SessionId}", session.SessionId); | |
| throw; | |
| } | |
| } | |
| public async Task<SessionInfo> UpdateAsync(SessionInfo session) | |
| { | |
| try | |
| { | |
| var key = GetSessionKey(session.SessionId); | |
| session.LastActivity = DateTime.UtcNow; | |
| var json = JsonSerializer.Serialize(session); | |
| var updated = await _database.StringSetAsync(key, json, SESSION_TTL, when: When.Exists); | |
| if (!updated) | |
| { | |
| _logger.LogWarning("세션 업데이트 실패(키 없음): {SessionId}", session.SessionId); | |
| throw new KeyNotFoundException($"세션을 찾을 수 없음: {session.SessionId}"); | |
| } | |
| - await _database.StringSetAsync(key, json, SESSION_TTL); | |
| _logger.LogDebug("세션 업데이트 성공 (하트비트): {SessionId}", session.SessionId); | |
| return session; | |
| } | |
| catch (Exception ex) | |
| { | |
| _logger.LogError(ex, "세션 업데이트 실패: {SessionId}", session.SessionId); | |
| throw; | |
| } | |
| } |
| public async Task DeleteAsync(string sessionId) | ||
| { | ||
| throw new NotImplementedException(); | ||
| try | ||
| { | ||
| var key = GetSessionKey(sessionId); | ||
| var deleted = await _database.KeyDeleteAsync(key); | ||
|
|
||
| if (deleted) | ||
| { | ||
| _logger.LogInformation("세션 삭제 성공: {SessionId}", sessionId); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogWarning("삭제할 세션을 찾을 수 없음: {SessionId}", sessionId); | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "세션 삭제 실패: {SessionId}", sessionId); | ||
| throw; | ||
| } | ||
| } |
There was a problem hiding this comment.
세션 삭제 시 인덱스에서 제거 누락 및 사용자 인덱스 정리 필요
Redis 키만 삭제하고 인덱스(Set)에서 sessionId를 제거하지 않으면 조회/카운트가 부정확해집니다. 또한 사용자 인덱스에서도 제거해야 합니다(가능하면 삭제 전 JSON에서 UserId를 파싱).
- var key = GetSessionKey(sessionId);
- var deleted = await _database.KeyDeleteAsync(key);
-
- if (deleted)
- {
- _logger.LogInformation("세션 삭제 성공: {SessionId}", sessionId);
- }
- else
- {
- _logger.LogWarning("삭제할 세션을 찾을 수 없음: {SessionId}", sessionId);
- }
+ var key = GetSessionKey(sessionId);
+ var value = await _database.StringGetAsync(key);
+ var deleted = await _database.KeyDeleteAsync(key);
+
+ if (deleted)
+ {
+ await _database.SetRemoveAsync(SESSIONS_INDEX_KEY, sessionId);
+ if (value.HasValue)
+ {
+ try
+ {
+ using var doc = JsonDocument.Parse(value);
+ if (doc.RootElement.TryGetProperty("UserId", out var userIdProp))
+ {
+ var uid = userIdProp.GetString();
+ if (!string.IsNullOrEmpty(uid))
+ await _database.SetRemoveAsync(USER_SESSIONS_INDEX_PREFIX + uid, sessionId);
+ }
+ }
+ catch (Exception jsonEx)
+ {
+ _logger.LogDebug(jsonEx, "세션 삭제 중 사용자 인덱스 제거를 위한 JSON 파싱 실패: {SessionId}", sessionId);
+ }
+ }
+ _logger.LogInformation("세션 삭제 성공: {SessionId}", sessionId);
+ }
+ else
+ {
+ _logger.LogWarning("삭제할 세션을 찾을 수 없음: {SessionId}", sessionId);
+ }Committable suggestion skipped: line range outside the PR's diff.
| public async Task<IEnumerable<SessionInfo>> GetSessionsByUserIdAsync(string? userId) | ||
| { | ||
| try | ||
| { | ||
| if (string.IsNullOrEmpty(userId)) | ||
| return Enumerable.Empty<SessionInfo>(); | ||
|
|
||
| var session = await GetAsync(userId); | ||
| return session != null ? new[] { session } : Enumerable.Empty<SessionInfo>(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "사용자 세션 조회 실패: {UserId}", userId); | ||
| return Enumerable.Empty<SessionInfo>(); | ||
| } | ||
| } |
There was a problem hiding this comment.
GetSessionsByUserIdAsync가 sessionId로 조회 — 기능 오동작
현재 userId를 sessionId로 착각하여 GetAsync(userId)를 호출합니다. 사용자당 다수 세션을 반환해야 하므로 사용자 인덱스(Set)를 사용해 조회해야 합니다.
- if (string.IsNullOrEmpty(userId))
- return Enumerable.Empty<SessionInfo>();
-
- var session = await GetAsync(userId);
- return session != null ? new[] { session } : Enumerable.Empty<SessionInfo>();
+ if (string.IsNullOrEmpty(userId))
+ return Enumerable.Empty<SessionInfo>();
+
+ var ids = await _database.SetMembersAsync(USER_SESSIONS_INDEX_PREFIX + userId);
+ if (ids.Length == 0)
+ return Enumerable.Empty<SessionInfo>();
+
+ var keys = new RedisKey[ids.Length];
+ for (int i = 0; i < ids.Length; i++)
+ keys[i] = (RedisKey)(SESSION_KEY_PREFIX + (string)ids[i]);
+
+ var values = await _database.StringGetAsync(keys);
+ var sessions = new List<SessionInfo>(values.Length);
+ foreach (var v in values)
+ {
+ if (!v.HasValue) continue;
+ var s = JsonSerializer.Deserialize<SessionInfo>(v.Span);
+ if (s != null) sessions.Add(s);
+ }
+ return sessions;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In ProjectVG.Infrastructure/Persistence/Session/RedisSessionStorage.cs around
lines 185-200, the method wrongly calls GetAsync(userId) treating userId as a
sessionId; change it to fetch the user's session ID set from Redis (the per-user
index/key that stores session IDs), return empty if userId is null/empty, then
for each sessionId in that set call GetAsync(sessionId) and aggregate non-null
SessionInfo objects (use Task.WhenAll or similar to fetch concurrently).
Preserve the try/catch and logging, and return Enumerable.Empty<SessionInfo>()
on error.
| _mockSessionManager.Setup(x => x.IsSessionActiveAsync(userId)) | ||
| .ReturnsAsync(sessionInfos.Any()); | ||
| } |
There was a problem hiding this comment.
세션 관리자 추가 호출 미설정으로 인한 NRE 위험
Validator는 GetActiveUserIdsAsync 결과를 바로 열거/Join합니다. 기본 Moq 리턴(null) 시 NRE 발생. 테스트 설정에 반환값을 추가하세요. 하트비트 호출도 no-op로 설정 권장.
_mockSessionManager.Setup(x => x.IsSessionActiveAsync(userId))
.ReturnsAsync(sessionInfos.Any());
+ _mockSessionManager.Setup(x => x.GetActiveSessionCountAsync())
+ .ReturnsAsync(1);
+ _mockSessionManager.Setup(x => x.GetActiveUserIdsAsync())
+ .ReturnsAsync(new[] { userId });
+ _mockSessionManager.Setup(x => x.UpdateSessionHeartbeatAsync(userId))
+ .Returns(Task.CompletedTask);
Summary by CodeRabbit