[FEAT#524] Enrich stage Redis ZSET intermediate queue — priority sub-ordering - #528
Conversation
…RetryJob (이슈 #524 Sub 1) Validate PR #527 의 동일 패턴 적용. ContentRef payload + DLQ fallback 모두 일관. 변경: - Worker.retryScheduler bus.RetryScheduler 필드 + SetRetryScheduler setter - Handle 분기: process 실패 시 - retryScheduler 주입 시: enqueueRetry → commit (메시지 손실 방지) - enqueue 실패 시 sendToDLQ fallback (DLQ reason 에 enqueue 에러 + 원본 process 에러 보존) - 미주입 시: 기존 commit skip → Kafka redeliver (Kafka 모드 호환) - BuildRetryJob: ProcessingMessage → ContentRef → CrawlJob 변환 - crawler/target_type 헤더 우선, "enrich-retry" fallback - PriorityFromHeader 헬퍼 / isValidTargetType 헬퍼 - timeout_ms 헤더 계승 (gemini PR #527 동일 정책) - retry_reason="enrich_process_failed" + original_ref_id metadata - MaxRetries = bus.DefaultMaxRetries Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Validate PR #527 의 zset_intake.go 패턴 복제. 차이점: - 입력 토픽: TopicValidated (Validate 는 TopicNormalized) - ZSET key 명명: "enrich:zset:queue" / "enrich:zset:entry:" (main.go wiring 단계) 흐름 (Validate 와 동일): - FetchMessage → ProcessingMessage unmarshal → ContentRef unmarshal - priority header → PriorityFromHeader - zsetQueue.Push(priority, ref.ID, payload) - Kafka commit (실패 시 idempotent — redeliver 시 동일 ID 재push) 실패 정책 동일: - unmarshal 실패 / 빈 ID: commit + skip - push 실패: commit skip (Kafka redeliver) API: - NewZSetIntake / Run (defer consumer.Close) / HandleOneForTest Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… Stage SetZSetIntake (이슈 #524 Sub 3) Validate PR #527 과 동일 패턴 — feature flag default off + STAGES_ENRICH_ENABLED 가드 + Redis + RetryScheduler 미설정 시 안전 fallback. 변경: - internal/processor/enrich/enrich.go: - Stage.intake *worker.ZSetIntake 필드 추가 (nil 허용) - SetZSetIntake setter - Start: intake 주입 시 go intake.Run(ctx) - cmd/issuetracker/main.go: - enrichPriorityQueueEnabled = stagesCfg.EnrichEnabled && env && redisClientShared - 모드 활성 시: PriorityZSetQueue + PriorityZSetConsumer + ZSetIntake 구성 - enrichConsumer 가 모드에 따라 zsetConsumer 또는 kafkaConsumer - ZSET 모드 + retryScheduler nil → fatal (메시지 손실 방지) - enrichStage.SetZSetIntake(enrichZSetIntake) 로 lifecycle 통합 - .env.example: ENRICH_PRIORITY_QUEUE_ENABLED + ENRICH_ZSET_* (Parser / Validate 동일 형식) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ke (이슈 #524 Sub 4) Validate PR #527 의 테스트 패턴 그대로 미러링 — ContentRef payload + DLQ 정책 모두 동일. BuildRetryJob 11 / ZSetIntake.handleOne 6 / PriorityFromHeader 9 cases. retry_reason="enrich_process_failed" / crawler fallback="enrich-retry" 검증. 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 (11)
📝 WalkthroughWalkthroughThis PR extends the Enrich stage with a Redis ZSET intermediate queue for priority-aware message processing. When enabled, validated Kafka messages are immediately routed to a ZSET, consumed by workers with priority ordering, and failed messages are retried via a configurable scheduler instead of relying on Kafka redelivery. ChangesEnrich Stage ZSET Priority Queue
Sequence Diagram(s)sequenceDiagram
participant KafkaConsumer as Kafka<br/>Consumer
participant ZSetIntake
participant PriorityZSetQueue
participant Worker
participant RetryScheduler
participant Enricher
KafkaConsumer->>ZSetIntake: FetchMessage (validated)
ZSetIntake->>PriorityZSetQueue: Push(contentID, priority)
PriorityZSetQueue-->>ZSetIntake: Enqueued
ZSetIntake->>KafkaConsumer: Commit offset
Worker->>PriorityZSetQueue: BZPOPMIN (priority-ordered)
PriorityZSetQueue-->>Worker: Message with highest priority
Worker->>Enricher: Process (LLM enrichment)
alt Enrichment succeeds
Enricher-->>Worker: Enriched content
Worker->>Worker: Commit/store result
else Enrichment fails
Enricher-->>Worker: error
Worker->>RetryScheduler: enqueueRetry(CrawlJob)
RetryScheduler-->>Worker: Scheduled for retry
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes The PR spans multiple files with dense logic in worker retry handling and ZSET intake flow control. Worker error branching requires careful review of commit semantics, header parsing, and job reconstruction. ZSET intake implements a subtle selective-commit strategy across multiple failure modes. Wiring in main.go introduces new conditionals and fatality checks. Comprehensive test coverage mitigates some complexity. The changes are heterogeneous (worker behavior, new component, stage integration, configuration) rather than repetitive refactoring. Possibly related PRs
Suggested labels
🚥 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/processor/enrich/enrich.go (1)
45-55:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
Stopdoes not own intake shutdown lifecycle
Startlaunchess.intake.Run(ctx)butStoponly stopss.worker. IfStopis called before the start-context is canceled, intake can keep consuming in the background.Suggested lifecycle fix
type Stage struct { worker *worker.Worker intake *worker.ZSetIntake // nil 허용 — ZSET 인입 모드일 때만 (이슈 `#524`) + intakeCancel context.CancelFunc + intakeDone chan struct{} } func (s *Stage) Start(ctx context.Context) { s.worker.Start(ctx) if s.intake != nil { - go s.intake.Run(ctx) + intakeCtx, cancel := context.WithCancel(ctx) + s.intakeCancel = cancel + s.intakeDone = make(chan struct{}) + go func() { + defer close(s.intakeDone) + s.intake.Run(intakeCtx) + }() } } func (s *Stage) Stop(ctx context.Context) error { + if s.intakeCancel != nil { + s.intakeCancel() + if s.intakeDone != nil { + select { + case <-s.intakeDone: + case <-ctx.Done(): + return ctx.Err() + } + } + } return s.worker.Stop(ctx) }🤖 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/enrich/enrich.go` around lines 45 - 55, The Stop method currently only stops s.worker but doesn't stop the intake goroutine started in Start (s.intake.Run(ctx)), so update Stop (on type Stage) to also shut down the intake when present: if s.intake != nil call the intake's shutdown method (e.g., s.intake.Stop(ctx) or signal/cancel the intake run) and wait for it to complete before/alongside s.worker.Stop(ctx); ensure you reference the same symbols (Stage.Stop, Stage.Start, s.intake.Run, s.worker.Stop) and choose the intake's existing stop/close API or add one if missing to properly own the intake lifecycle.
🤖 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.
Outside diff comments:
In `@internal/processor/enrich/enrich.go`:
- Around line 45-55: The Stop method currently only stops s.worker but doesn't
stop the intake goroutine started in Start (s.intake.Run(ctx)), so update Stop
(on type Stage) to also shut down the intake when present: if s.intake != nil
call the intake's shutdown method (e.g., s.intake.Stop(ctx) or signal/cancel the
intake run) and wait for it to complete before/alongside s.worker.Stop(ctx);
ensure you reference the same symbols (Stage.Stop, Stage.Start, s.intake.Run,
s.worker.Stop) and choose the intake's existing stop/close API or add one if
missing to properly own the intake lifecycle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2115e426-f04c-4416-bc3e-6645aa8208b9
📒 Files selected for processing (7)
.env.examplecmd/issuetracker/main.gointernal/processor/enrich/enrich.gointernal/processor/enrich/worker/worker.gointernal/processor/enrich/worker/zset_intake.gotest/internal/processor/enrich/worker/retry_job_test.gotest/internal/processor/enrich/worker/zset_intake_test.go
There was a problem hiding this comment.
Code Review
This pull request introduces a Redis ZSET-based priority queue for the Enrich stage, enabling priority sub-ordering by bypassing Kafka's partition FIFO constraints. Key changes include the implementation of a ZSetIntake component to move messages from Kafka to Redis, the addition of a RetryScheduler to the Enrich worker to prevent message loss in ZSET mode, and the necessary wiring in the main entry point. Feedback highlights the need to ensure at-least-once processing by avoiding offset commits if retry or DLQ operations fail, the recommendation to use context.WithoutCancel for terminal operations, and the opportunity to refactor the duplicated PriorityFromHeader logic into a common utility package.
gemini #3278202670 (Medium) DRY 위반 반영. Parser / Validate / Enrich 의 동일 함수
3개를 pkg/queue.PriorityFromHeader 로 통합.
변경:
- pkg/queue/priority_header.go (신규):
- PriorityFromHeader(headers) int — 단일 출처
- PriorityHeaderKey 상수 ("priority")
- internal/processor/{parser,validate,enrich}/worker/:
- 각 패키지의 PriorityFromHeader 정의 제거 + 이관 안내 주석
- 호출처를 queue.PriorityFromHeader 로 변경
- strconv import 제거 (parser/zset_intake.go)
- test/pkg/queue/priority_header_test.go (신규): 9 cases 단일 테스트
- 각 worker package 의 중복 TestPriorityFromHeader_Mapping 제거
gemini #3278202667 (HIGH) at-least-once 보장은 이미 c10b4b5 / Enrich PR 본문에
구현됨 (enqueueRetry 실패 → DLQ fallback → 실패 시 commit X). timeout_ms 헤더
계승도 BuildRetryJob 에 이미 포함. context.WithoutCancel 패치는 Parser/Validate
와의 일관성을 고려해 본 PR scope 외로 분리.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
구현 내용
메타 이슈 #515 의 Phase 2 마지막 stage — Validate #527 패턴을 Enrich 에 미러링. Kafka partition FIFO 가 priority sub-ordering 을 제공 못 하는 한계를 Redis ZSET 으로 해소.
흐름 (Validate 동일):
Sub 1 — Enrich Worker retry hook (7830db9)
Worker.retryScheduler필드 +SetRetrySchedulersetterHandle분기: process 실패 → enqueueRetry → DLQ fallback → commitBuildRetryJob: ProcessingMessage → ContentRef → CrawlJob 변환PriorityFromHeader헬퍼 /isValidTargetType헬퍼timeout_ms헤더 계승retry_reason="enrich_process_failed"+original_ref_idmetadataSub 2 — Kafka → ZSET intake goroutine (1f4768f)
worker.ZSetIntake— Validate 동일 패턴, 입력 토픽만TopicValidated로 변경Sub 3 — Feature flag + main.go wiring + Stage 통합 (e59474d)
ENRICH_PRIORITY_QUEUE_ENABLED=true+STAGES_ENRICH_ENABLED=true+redisClientShared != nil시 ZSET 모드 활성enrich.Stage.SetZSetIntake로 lifecycle 통합ENRICH_ZSET_QUEUE_KEY/ENRICH_ZSET_ENTRY_PREFIX/ENRICH_ZSET_MAX_SIZE/ENRICH_ZSET_ENTRY_TTL/ENRICH_ZSET_POP_TIMEOUT.env.example갱신Sub 4 — 단위 테스트 (이번 커밋)
CI / 머지 게이트 점검
변경 영향 범위
internal/processor/enrich,cmd/issuetracker,.env.exampleRequired Status Checks
Commit LintPR Title LintLinked Issue CheckFormat CheckBuildTestLint롤백 계획
ENRICH_PRIORITY_QUEUE_ENABLED=false(또는 unset) — 기존 Kafka consume 흐름 복귀메타 이슈 #515 Phase 2 완료
본 PR 머지 시 메타 이슈 #515 의 모든 sub-issue (#521 / #522 / #523 / #524) 완료. Publisher 단계 host/path priority 분기 (Phase 1) + Parser / Validate / Enrich 의 ZSET priority sub-ordering (Phase 2) 인프라가 모두 갖춰집니다.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Configuration