Conversation
SeoYanoo
changed the title
| this.refreshFunc = null | ||
| } | ||
| }, | ||
| pollContestRank() { |
There was a problem hiding this comment.
pollContestRank가 진입 시점에 stopRankPolling()을 호출해 refreshFunc를 null로 만듭니다.
그래서 요청이 떠 있는 동안 컴포넌트가 파괴되면, beforeDestroy의 stopRankPolling()은
지울 타이머가 없어 아무 일도 하지 않습니다.
이후 진행 중이던 요청이 끝나면서 .finally의 startRankPolling()이 실행되는데,
이 시점의 this는 이미 파괴된 인스턴스입니다. document.hidden은 false이고
CLEAR_CONTEST 이후 contestStatus가 null이라 refreshDisabled도 false여서
새 타이머가 아무 저항 없이 걸립니다.
결과적으로 랭킹 탭을 열었다 나갈 때마다 정리되지 않는 2초 주기 요청 루프가
하나씩 쌓입니다. 게다가 요청 대상은 사용자가 새로 이동한 대회가 됩니다.
재현: 랭킹 탭에 들어가자마자 다른 탭으로 이동한 뒤 네트워크 패널을 보면
getContestRank가 계속 나갑니다. 몇 번 왕복하면 요청 수가 배로 늘어납니다.
.finally 안에서 "이 컴포넌트가 아직 살아 있는가"를 확인할 방법이 필요해 보입니다.
| }, | ||
| refreshDisabled() { | ||
| return this.contest.status === CONTEST_STATUS.ENDED | ||
| return this.contestStatus === CONTEST_STATUS.ENDED |
There was a problem hiding this comment.
아직 시작하지 않은 대회에서도 폴링이 돕니다.
refreshDisabled 가 지금은 종료된 대회만 막습니다. 시작 전 대회는 안 막습니다.
비밀번호가 걸린 대회에서 사용자가 비밀번호를 입력하면 랭킹 탭에 들어갈 수 있습니다.
대회가 아직 시작 전이어도요.
이때 서버는 매번 "Contest has not started yet." 을 돌려줍니다.
updateContestData 가 이 에러를 조용히 삼키고, .finally 가 2초 뒤를 다시 예약합니다.
그래서 화면에는 아무 표시 없이 초당 2번씩 실패 요청이 계속 나갑니다.
사용자는 모르고 서버 로그에만 쌓입니다.
폴링이 돌아야 하는 조건이 "종료 안 됨" 이 맞는지,
아니면 "진행 중" 이어야 하는지 한번 봐주세요.
| if cached_rank is not None: | ||
| return cached_rank | ||
| # Delete first so a failed SET cannot leave an old snapshot active. | ||
| cache.delete(cache_key) |
There was a problem hiding this comment.
재빌드 직전의 cache.delete 가 오히려 부하를 만듭니다.
주석에는 SET 이 실패했을 때 옛 데이터가 남지 않도록 하기 위해서라고 되어 있습니다.
그런데 SET 은 덮어쓰기라서 실패하면 옛 데이터가 그대로 남을 뿐이고,
키에는 이미 TTL 이 있어서 어차피 사라집니다.
이 delete 가 막아주는 상황이 없습니다.
반면 부작용은 확실합니다. 채점이 끝날 때마다 캐시가 잠깐 비는 구간이 생깁니다.
그 사이에 들어온 조회 요청은 전부 캐시 미스가 납니다.
그러면 각자 refresh_public_rank_cache 를 불러서 같은 락 앞에 줄을 섭니다.
참가자 50명이 2초마다 폴링하면 초당 25요청입니다 (PR 설명의 추정치).
gunicorn 은 워커당 스레드가 4개입니다.
25개 요청이 각각 최대 2초씩 스레드를 잡고 기다리면 스레드가 모자랍니다.
2초 안에 락을 못 잡은 요청은 뷰의 if qs is None 쪽으로 빠집니다.
거기서 각자 전체 순위를 DB 에서 다시 읽습니다.
캐시로 막으려던 DB 몰림이 바로 여기서 일어납니다.
delete 를 빼면 재빌드 중에도 이전 순위를 계속 보여줄 수 있는데,
이 delete 가 꼭 필요한지 확인 부탁드립니다.
| transaction.on_commit( | ||
| lambda current_contest_id=contest_id: refresh_public_rank_cache(current_contest_id, force=True) | ||
| ) | ||
|
|
There was a problem hiding this comment.
채점 한 건마다 전체 순위를 다시 만듭니다.
기존에는 채점이 끝나면 cache.delete(key) 하나였습니다. Redis 명령 한 번입니다.
지금은 refresh_public_rank_cache(contest_id, force=True) 가 돕니다.
이 안에서 Contest 를 다시 조회하고, 참가자 전체를 select_related 로 가져오고,
DRF 로 전부 직렬화하고, json.dumps 까지 합니다. 제출 한 건마다요.
대회 중에는 여러 채점이 동시에 끝납니다.
그러면 채점 쪽끼리도 락 앞에서 서로 기다립니다 (blocking_timeout=2).
락을 못 잡은 채점은 바깥 except Exception 에 걸려서 갱신을 그냥 건너뜁니다.
에러도 안 납니다.
그러면 순위는 결국 30초 TTL 이 끝날 때까지 낡은 채로 남습니다.
비용은 치렀는데 실시간성은 못 얻는 경우가 생깁니다.
부하가 가장 심한 순간에 채점이 느려지는 구조입니다.
갱신을 조회하는 쪽에 맡기거나, 여러 채점을 묶어서 한 번만 갱신하는 방법도 있을 것 같습니다.
| if download_csv: | ||
| if not is_contest_admin: | ||
| return self.error("No permission to download contest rank") | ||
| data = serializer(qs, many=True, is_contest_admin=is_contest_admin).data |
There was a problem hiding this comment.
권한 없는 요청도 순위를 다 만든 다음에 거절합니다.
지금 순서가 이렇습니다.
- get_public_rank 로 순위를 가져옵니다.
캐시에 없으면 DB 에서 전부 읽고 직렬화합니다. - 그 다음에 download_csv 권한을 확인합니다.
- 권한이 없으면 방금 만든 결과를 버리고 에러를 돌려줍니다.
권한 없는 사람이 ?download_csv=1 을 반복해서 호출하면
매번 전체 순위 재빌드가 일어날 수 있습니다. 공격 비용이 거의 들지 않습니다.
권한 검사가 지금 위치에 있어야 할 이유가 있는지 확인 부탁드립니다.
| self.assertEqual(data, cached_data) | ||
| rank_cache.delete.assert_not_called() | ||
| rank_cache.set.assert_not_called() | ||
|
|
There was a problem hiding this comment.
이 테스트가 지금은 아무것도 검증하지 못합니다.
get_public_rank 를 목으로 바꾸고, 반환값으로 ACMContestRankSerializer(...).data 를 넣었습니다.
그리고 응답의 user 키가 {id, username, avatar} 인지 확인합니다.
그런데 그 데이터를 만든 게 바로 그 serializer 입니다.
serializer 가 그 세 개만 내놓으니 결과도 당연히 그 세 개입니다. 항상 통과합니다.
나중에 get_public_rank 나 refresh_public_rank_cache, serialize_public_rank 중 하나가
관리자용 serializer 를 쓰게 되거나 필드를 흘려도 이 테스트는 그대로 통과합니다.
정작 잡아야 할 상황을 못 잡습니다.
이 PR 의 핵심이 비공개 필드 보호인데 그 부분이 테스트로 보호되지 않습니다.
목을 걷어내고 실제 코드가 돌아가게 하면 의미가 생길 것 같습니다.
This branch has not been deployed
2 participants
Changelog
select_related를 적용해 순위 사용자별 추가 쿼리가 발생하지 않도록 개선합니다.Closes #801
Related to #612
Testing
git diff --check통과Ops Impact
contest_rank_cache:v2:*형식의 새로운 캐시 키가 추가됩니다.contest_rank_cache:*키는 더 이상 순위 조회에 사용되지 않으며 필요하면 별도로 정리할 수 있습니다.Version Compatibility
id,username,avatar는 유지됩니다.email,student_id,school,major,admin_type등 불필요한 필드는 제거됩니다.