[FEAT#522] Parser stage Redis ZSET intermediate queue — priority sub-ordering - #526
Conversation
…522 Sub 1) Redis ZSET 기반 priority queue 도입 — Kafka partition FIFO 가 priority sub-ordering 을 제공 못 하는 한계를 해소하기 위한 intermediate queue 인프라. 설계: - score = priority(1=high/2=normal/3=low) × 1e10 + arrival_timestamp_ms - 1e10 priority 간격이 arrival_ts (UnixMilli, ~1.7e12) 보다 작아 priority 가 dominant - high 가 항상 normal/low 보다 먼저 pop, 같은 priority 안에서는 timestamp FIFO - float64 mantissa 한계 2^53 이내 — 정밀도 손실 없음 - ZADD + SET pipeline 으로 1 RTT push (retry_queue 패턴 동일) - BZPOPMIN 으로 atomic pop + entry GET / DEL — Kafka FetchMessage 와 동일 blocking 의미 - maxSize 초과 시 ZREMRANGEBYRANK 로 가장 큰 score (low priority + 오래된) 항목 drop - 동일 ID 재push 시 ZSET 덮어쓰기 (idempotent for retry) API: - PriorityZSetQueue.Push(ctx, priority, id, payload) — pipeline ZADD + SET - PriorityZSetQueue.Pop(ctx, timeout) (*PopResult, error) — BZPOPMIN + entry GET - PriorityZSetQueue.Len(ctx) — ZCard - PriorityZSetConsumer (queue.Consumer 어댑터): - FetchMessage: Pop 결과를 Message 로 변환, "priority" header 부착 - CommitMessages: no-op (pop = ack) - Close: no-op (redis 외부 관리) 테스트 (Redis 통합, 미가용 시 skip): - Push/Pop roundtrip / priority order / FIFO within same priority / empty timeout - ctx cancel / empty id / empty payload / Len / duplicate ID overwrite / invalid priority - Consumer FetchMessage / Commit no-op / Close no-op / ctx cancel 후속 (#522): Parser worker 가 본 추상화를 통해 Kafka→ZSET 인입 + ZSET→worker pop 흐름으로 전환. 재사용 예정: Validate (#523) / Enrich (#524). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#522 Sub 2) ZSET 인입 모드에서는 BZPOPMIN 이 곧 ack (pop=remove) 이라 commit skip 으로 redeliver 가 불가. RetryScheduler 경유로 Kafka 재발행 → 다음 intake 가 ZSET 으로 흡수하는 패턴이 메시지 손실 방지의 유일한 방법. 변경: - Worker.retryScheduler bus.RetryScheduler 필드 추가 (nil 허용) - SetRetryScheduler setter — Start 전 wiring 단계에서 1회 주입 - Handle: ProcessMessage 실패 시 retryScheduler 주입 여부 분기 - 주입 시: enqueueRetry → commit (메시지 손실 방지) - 미주입 시: 기존 commit skip → Kafka redeliver (Kafka 모드 호환) - enqueueRetry helper: RawContentRef → CrawlJob 변환 - URL / CrawlerName / priority (header) 보존 - Target.Type=Article, retry_reason / original_raw_id metadata 부착 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ZSetIntake: 단일 goroutine 이 TopicFetched 메시지를 fetch → priority + arrival timestamp 로 score 계산 후 ZSET 적재 + Kafka commit. Worker pool 은 ZSET 에서 pop 하여 처리 (별도 흐름) — Kafka partition FIFO 제약 우회. 실패 정책: - Unmarshal 실패: 형식 깨진 데이터 — commit + skip (재시도 무의미) - 빈 RawContentRef.ID: 동일하게 commit + skip - ZSET push 실패: Redis 일시 장애 — commit skip 으로 Kafka redeliver - Kafka commit 실패: ZSET 에 이미 push 됨, redeliver 시 동일 ID 재push (idempotent) API: - NewZSetIntake(consumer, zsetQueue, log) *ZSetIntake — nil 인자 시 nil 반환 - Run(ctx) — blocking, ctx cancel 시 종료 - handleOne 분리 — 단위 테스트 용이성 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#522 Sub 4) ZSET 인입 모드를 환경변수로 토글. redisClientShared 미설정 / RetryScheduler nil 환경 에서는 자동 비활성 — 안전 fallback. 변경: - internal/processor/parser/stage.go: - Stage.intake *worker.ZSetIntake 필드 추가 (nil 허용) - Stage.SetZSetIntake setter - Stage.Start: nil 아니면 go intake.Run(ctx) - cmd/issuetracker/main.go: - envBoolOrDefault / envOrDefault 헬퍼 신설 - parserPriorityQueueEnabled 분기: - true 시: PriorityZSetQueue + PriorityZSetConsumer + ZSetIntake 구성 - false 시: 기존 Kafka consumer 직접 사용 - parserConsumer 가 모드에 따라 zsetConsumer 또는 kafkaConsumer - ZSET 모드 활성 시 retryScheduler 필수 (nil 이면 fatal — 메시지 손실 방지) - parserStg.SetZSetIntake(parserZSetIntake) 로 lifecycle 연동 환경변수: - PARSER_PRIORITY_QUEUE_ENABLED (bool, default false) - PARSER_ZSET_QUEUE_KEY (string, default "parser:zset:queue") - PARSER_ZSET_ENTRY_PREFIX (string, default "parser:zset:entry:") - PARSER_ZSET_MAX_SIZE (int, default 100000) - PARSER_ZSET_ENTRY_TTL (duration, default 24h) - PARSER_ZSET_POP_TIMEOUT (duration, default 1s) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
테스트 용이성 위해 buildRetryJob → BuildRetryJob, priorityFromHeader → PriorityFromHeader export. 외부 stable API 가 아닌 내부 helper 지만 unit test 접근 위해 노출. 테스트 케이스 (16): - BuildRetryJob 7 cases: - 정상 RawContentRef → CrawlJob (priority/url/crawler/metadata 검증) - 헤더 없음 → normal default - 잘못된 priority 값 (0/4/abc/-1) → normal 보정 - 빈 source name → "parser-retry" fallback - 빈 URL → error - malformed JSON → error - PriorityFromHeader 9 cases: - 1/2/3 정상 매핑 - missing/empty/non-numeric/out-of-range (0,4,-1) → 2 (normal) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR implements an optional Redis ZSET-backed priority queue for the parser stage, allowing priority-aware message processing independent of Kafka partition ordering. When enabled via env flag, Kafka messages are bridged into a Redis ZSET, popped by priority + arrival order, with retry-scheduler integration for fail-safe redelivery without message loss. ChangesParser Priority Queue via Redis ZSET
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Code Review
This pull request introduces a Redis ZSET-based priority queueing mechanism for the parser stage to enable priority-based processing that bypasses Kafka's partition FIFO constraints. Key changes include the addition of a ZSetIntake component for transferring messages from Kafka to Redis, a PriorityZSetQueue implementation, and logic within the worker to utilize a RetryScheduler for preventing message loss. Feedback focuses on a critical priority inversion bug caused by an insufficient priorityFactor, hardcoded target types in retry jobs that could break category processing, and the need for better consistency in crawler name resolution and code reuse.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/processor/parser/worker/worker.go (1)
328-343: 💤 Low valueConsider logging the original error when retry enqueue fails.
When
enqueueRetryfails (line 331-333), the log message includesenqueueErrbut not the original processing errorerr. In ZSET mode, the message will be lost without a record of why it originally failed.💡 Proposed enhancement for debugging visibility
if w.retryScheduler != nil { if enqueueErr := w.enqueueRetry(ctx, msg, err); enqueueErr != nil { - log.WithError(enqueueErr).WithField("offset", msg.Offset).Warn("retry enqueue failed, message will be lost in zset mode") + log.WithError(enqueueErr).WithFields(map[string]interface{}{ + "offset": msg.Offset, + "original_error": err.Error(), + }).Warn("retry enqueue failed, message will be lost in zset mode") return }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/processor/parser/worker/worker.go` around lines 328 - 343, When enqueueRetry fails in the w.retryScheduler branch, the warning currently logs only enqueueErr and not the original processing error (err); update the log call in the failing branch inside the if w.retryScheduler != nil block so it includes both enqueueErr and the original err (and keep the msg.Offset field) to preserve the root cause for ZSET-mode message loss debugging; locate the enqueueRetry call and its enclosing warning log and add the original err to the log context (e.g., WithError(err) or equivalent) while retaining the existing enqueueErr detail.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/processor/parser/worker/worker.go`:
- Around line 328-343: When enqueueRetry fails in the w.retryScheduler branch,
the warning currently logs only enqueueErr and not the original processing error
(err); update the log call in the failing branch inside the if w.retryScheduler
!= nil block so it includes both enqueueErr and the original err (and keep the
msg.Offset field) to preserve the root cause for ZSET-mode message loss
debugging; locate the enqueueRetry call and its enclosing warning log and add
the original err to the log context (e.g., WithError(err) or equivalent) while
retaining the existing enqueueErr detail.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2c18c4f7-1e13-46de-bf5f-f5b7de2f3c2c
📒 Files selected for processing (7)
cmd/issuetracker/main.gointernal/processor/parser/stage.gointernal/processor/parser/worker/worker.gointernal/processor/parser/worker/zset_intake.gopkg/queue/priority_zset.gotest/internal/processor/parser/worker/retry_job_test.gotest/pkg/queue/priority_zset_test.go
…t_type 보존 gemini 4건 피드백 반영: #3274689255 (HIGH) priorityFactor 1e10 → 1e13: - arrival_ts (UnixMilli, ~1.7e12) 가 priorityFactor (1e10) 보다 커서 정렬에서 우세하던 문제 - 1e13 으로 두면 priority 간 차이 (1e13) 가 ts 변동 (~1e12) 보다 ~10x dominant - float64 mantissa 한계 9e15 — priority=3 * 1e13 + ts ≈ 3.018e13, 정밀도 안전 #3274689285 (HIGH) target_type 헤더 보존: - 기존: Target.Type=core.TargetTypeArticle 하드코딩 → category 페이지 retry 시 article 오인 - 변경: msg.Headers["target_type"] 가 유효 (article/category) 면 사용, 없거나 잘못되면 Article default - isValidTargetType 헬퍼 + 단위 테스트 #3274689296 (Medium) priority 파싱 중복 제거: - BuildRetryJob 의 priority 파싱이 PriorityFromHeader 와 중복 → 후자 재사용 #3274689308 (Medium) crawler 헤더 우선: - msg.Headers["crawler"] 우선, 없으면 RawContentRef.SourceInfo.Name, 둘 다 빈 값 시 "parser-retry" - ProcessMessage 의 헤더 우선 정책과 일관 테스트 추가 (BuildRetryJob 5 신규): - crawler 헤더 우선 / 없을 때 SourceInfo fallback - target_type=category / =article / invalid → Article fallback Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
이 PR은 Parser stage에서 Kafka partition FIFO만으로는 제공하기 어려운 priority sub-ordering을 보장하기 위해, Redis ZSET intermediate queue를 도입하고(feature flag로 opt-in), 실패 시 RetryScheduler 경유 재발행 흐름을 추가합니다. 전체적으로 Phase 2 Parser stage의 처리 우선순위 일관성을 다중 인스턴스 환경까지 확장하는 변경입니다.
Changes:
pkg/queue.PriorityZSetQueue+PriorityZSetConsumer추가(푸시/팝/길이, workerpool 재사용)- Parser stage에 Kafka→ZSET intake goroutine 추가 및
Worker의 RetryScheduler 기반 실패 처리 훅 추가 cmd/issuetracker/main.go에 feature flag/env wiring 및 관련 단위/통합 테스트 추가
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/queue/priority_zset.go |
Redis ZSET 기반 priority 큐/Consumer 구현 추가 |
internal/processor/parser/worker/zset_intake.go |
Kafka 메시지 intake 후 ZSET 적재 + Kafka commit 루프 추가 |
internal/processor/parser/worker/worker.go |
ProcessMessage 실패 시 RetryScheduler enqueue 경로 및 BuildRetryJob 추가 |
internal/processor/parser/stage.go |
Stage lifecycle에 ZSetIntake 실행(go routine) 통합 |
cmd/issuetracker/main.go |
PARSER_PRIORITY_QUEUE_ENABLED 기반 ZSET 모드 wiring, env helper 추가 |
test/pkg/queue/priority_zset_test.go |
PriorityZSetQueue/Consumer Redis 통합 테스트 추가 |
test/internal/processor/parser/worker/retry_job_test.go |
BuildRetryJob/PriorityFromHeader 단위 테스트 추가 |
… 단위 테스트 + .env.example Copilot 8건 피드백 (#3274731261 은 이미 3ab9a0a 에서 해결) 반영: #3274731302 (HIGH) Pop GET 실패 메시지 손실 방지: - BZPOPMIN 후 entry GET 이 Redis 오류 (timeout/network) 로 실패하면 메시지 영구 손실 - 동일 score 로 ZADD 복구 시도 → caller 가 재시도 가능 - 복구도 실패 시 진짜 손실로 분류하여 명시적 error 반환 #3274731405 (HIGH) parserKafkaConsumer 리소스 누수: - ZSET 모드에서 parserKafkaConsumer 가 intake 전용이라 Worker.Stop 의 zsetConsumer Close 만으로 Kafka reader 자원 정리 안 됨 - ZSetIntake.Run 종료 시 defer consumer.Close() 호출로 자연 cleanup #3274731323 (Med) MaxSize 주석 정합: - 주석은 "0=unlimited" 였으나 코드는 "0=default" — 주석을 코드 동작에 맞춤 - unlimited 옵션은 의도적 미지원 (Redis 메모리 안전망) #3274731364 (Med) MaxRetries 매직 넘버: - BuildRetryJob 의 MaxRetries:3 → bus.DefaultMaxRetries (chain.go 와 일관) #3274731563 (Med) ZSetIntake 단위 테스트 부재: - PriorityPusher 인터페이스 export (Push only) — ZSetIntake.zsetQueue 가 인터페이스 의존 - *PriorityZSetQueue 가 자동 만족 + 테스트에서 stub 으로 교체 - ZSetIntake.HandleOneForTest export (handleOne 의 테스트 wrapper) - 단위 테스트 5건: 정상 push+commit / unmarshal 실패 commit / 빈 ID commit / push 실패 commit skip / 헤더 없을 때 normal default #3274731453 (Low) .env.example 신규 키: - PARSER_PRIORITY_QUEUE_ENABLED + PARSER_ZSET_* 환경변수 추가 + 운영 가이드 주석 #3274731503 (Low) 테스트 cleanup iter.Err: - SCAN iterator 의 iter.Err() 확인 (silent 실패 회피) - entry 만료 케이스 단위 테스트 추가 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
구현 내용
메타 이슈 #515 의 Phase 2 Parser stage. Kafka partition FIFO 가 priority sub-ordering 을 제공 못 하는 한계를 Redis ZSET intermediate queue 로 해소. PR #511 / #516 의 Redis 패턴 재사용.
흐름:
Sub 1 —
pkg/queue.PriorityZSetQueue추상화 (fd91b3c)Push(ctx, priority, id, payload)— ZADD + SET pipeline (retry_queue 동일 패턴)Pop(ctx, timeout)— BZPOPMIN + entry GET / DEL (atomic pop = ack)Len(ctx)— ZCardPriorityZSetConsumer—queue.Consumer어댑터 → workerpool.ConsumerPool 재사용Sub 2 — Parser Worker RetryScheduler hook (3e5e285)
Worker.retryScheduler bus.RetryScheduler필드 +SetRetrySchedulersetterHandle분기: ProcessMessage 실패 시enqueueRetry후 commit (메시지 손실 방지)BuildRetryJob(msg) (*core.CrawlJob, error)헬퍼 (export for test)Sub 3 — Kafka → ZSET intake goroutine (b724732)
worker.ZSetIntake단일 goroutinePriorityFromHeader(headers)헬퍼 (export for test)Sub 4 — Feature flag + main.go wiring (370b0ff)
PARSER_PRIORITY_QUEUE_ENABLED=true+redisClientShared != nil시 ZSET 모드 활성parser.Stage.SetZSetIntake로 lifecycle 통합 (Stage.Start 시 go intake.Run(ctx), ctx cancel 시 자연 종료)envBoolOrDefault/envOrDefault헬퍼 추가Sub 5 — 단위 테스트 (134b3db)
CI / 머지 게이트 점검
변경 영향 범위
pkg/queue,internal/processor/parser/worker,internal/processor/parser,cmd/issuetrackerRequired Status Checks
Commit LintPR Title LintLinked Issue CheckFormat CheckBuildTestLint롤백 계획
PARSER_PRIORITY_QUEUE_ENABLED=false(또는 unset) — 기존 Kafka 직접 consume 흐름 복귀후속 작업 (메타 #515 Phase 2)
pkg/queue.PriorityZSetQueue추상화 활용)🤖 Generated with Claude Code
Summary by CodeRabbit