[FEAT#523] Validate stage Redis ZSET intermediate queue — priority sub-ordering - #527
Conversation
…ldRetryJob (이슈 #523 Sub 1) Parser PR #526 의 동일 패턴 적용 — ZSET 인입 모드에서 BZPOPMIN 이 곧 ack 라 commit skip 으로 redeliver 불가. RetryScheduler 경유로 Kafka 재발행 → 다음 intake 가 ZSET 으로 흡수하는 패턴이 메시지 손실 방지의 유일한 방법. 변경: - Worker.retryScheduler bus.RetryScheduler 필드 추가 (nil 허용) - SetRetryScheduler setter — Start 전 wiring 단계에서 1회 주입 - Handle: process 실패 시 retryScheduler 주입 여부 분기 - 주입 시: enqueueRetry → commit (메시지 손실 방지) - 미주입 시: 기존 commit skip → Kafka redeliver (Kafka 모드 호환) - BuildRetryJob: ProcessingMessage → ContentRef → CrawlJob 변환 - msg.Headers crawler / target_type 우선, ContentRef.SourceInfo fallback - PriorityFromHeader 헬퍼로 priority parsing 통일 - retry_reason="validate_process_failed" / original_ref_id metadata - MaxRetries = bus.DefaultMaxRetries - PriorityFromHeader / isValidTargetType 헬퍼 (Parser 의 동일 이름 함수와 1:1) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Parser PR #526 의 zset_intake.go 패턴 복제. 차이점: - 입력 토픽: TopicNormalized - payload 구조: ProcessingMessage 한 단계 wrap + Data 필드의 ContentRef - ZSET member key: ContentRef.ID (Parser 는 RawContentRef.ID) 흐름 (Parser 와 동일): - FetchMessage → ProcessingMessage unmarshal → ContentRef unmarshal - priority header → PriorityFromHeader - zsetQueue.Push(priority, ref.ID, payload) - Kafka commit (실패 시 idempotent — redeliver 시 동일 ID 재push) 실패 정책 (동일): - ProcessingMessage / ContentRef unmarshal 실패: commit (재시도 무의미) - 빈 ref.ID: commit + skip - ZSET push 실패: commit skip (Kafka redeliver) API: - NewZSetIntake(consumer, zsetQueue, log) *ZSetIntake — nil 인자 시 nil 반환 - Run(ctx) — blocking, defer consumer.Close() 로 자원 정리 - HandleOneForTest 단위 테스트 export Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… + Stage SetZSetIntake (이슈 #523 Sub 3) Parser PR #526 과 동일 패턴 — feature flag default off, Redis + RetryScheduler 미설정 시 안전 fallback. 변경: - internal/processor/validate/stage.go: - Stage.intake *worker.ZSetIntake 필드 추가 (nil 허용) - SetZSetIntake setter - Start: intake 주입 시 go intake.Run(ctx) - cmd/issuetracker/main.go: - validatePriorityQueueEnabled 분기 - true 시: PriorityZSetQueue + PriorityZSetConsumer + ZSetIntake 구성 - false 시: 기존 Kafka consumer 직접 사용 - validateConsumer 가 모드에 따라 zsetConsumer 또는 kafkaConsumer - ZSET 모드 + retryScheduler nil → fatal (메시지 손실 방지) - validateStage.SetZSetIntake(validateZSetIntake) 로 lifecycle 통합 - .env.example: VALIDATE_PRIORITY_QUEUE_ENABLED + VALIDATE_ZSET_* (Parser 동일 형식) 환경변수: - VALIDATE_PRIORITY_QUEUE_ENABLED (default false) - VALIDATE_ZSET_QUEUE_KEY (default "validate:zset:queue") - VALIDATE_ZSET_ENTRY_PREFIX (default "validate:zset:entry:") - VALIDATE_ZSET_MAX_SIZE (default 100000) - VALIDATE_ZSET_ENTRY_TTL (default 24h) - VALIDATE_ZSET_POP_TIMEOUT (default 1s) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…take (이슈 #523 Sub 4) Parser PR #526 와 동일 패턴 단위 테스트: BuildRetryJob 11 cases: - 정상 ContentRef → CrawlJob (priority/url/crawler/metadata 검증) - 헤더 없음 → normal default / 잘못된 priority → normal 보정 - crawler 헤더 우선 / 없을 때 SourceInfo fallback / 둘 다 빈 값 → "validate-retry" - target_type=category / =article / invalid → Article fallback - 빈 URL / malformed ProcessingMessage / malformed ContentRef → error ZSetIntake.HandleOneForTest 6 cases (PriorityPusher + stubConsumer mock): - 정상 push+commit - ProcessingMessage unmarshal 실패 → commit - ContentRef unmarshal 실패 → commit - 빈 ID → commit - push 실패 → commit skip (Kafka redeliver) - 헤더 없을 때 normal priority default PriorityFromHeader 9 cases: Parser 와 동일 매핑 검증. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughValidate stage now supports optional Redis ZSET-based intake for priority-aware message processing. Kafka messages are conditionally bridged into a ZSET, workers pop by priority, and failed validations are retried by reconstructing job metadata from message headers. ChangesValidate ZSET Intake and Retry Flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 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 docstrings
🧪 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 implements a priority queue for the validation stage using Redis ZSET, enabling priority-based sub-ordering of messages by moving them from Kafka to an intermediate Redis queue. Key changes include the introduction of a ZSetIntake component to handle the message transfer, lifecycle management for this process within the validation stage, and the integration of a RetryScheduler to prevent message loss during processing failures in ZSET mode. Feedback suggests improving the BuildRetryJob function by inheriting the timeout value from message headers instead of using a hardcoded 30-second default, which would ensure consistency with existing republishing logic.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@cmd/issuetracker/main.go`:
- Around line 1052-1055: The validate-only initialization is being enabled even
when the Validate stage is globally disabled; update the gating logic so
validatePriorityQueueEnabled also requires stagesCfg.ValidateEnabled (i.e.,
combine envBoolOrDefault("VALIDATE_PRIORITY_QUEUE_ENABLED", false) &&
redisClientShared != nil && stagesCfg.ValidateEnabled) and apply the same change
to the duplicate block around validateKafkaConsumer/validateZSetIntake (the
other occurrence referenced in the review), ensuring both
validatePriorityQueueEnabled declarations and any conditionals that instantiate
validateKafkaConsumer or validateWorkerPkg.ZSetIntake honor
stagesCfg.ValidateEnabled.
In `@internal/processor/validate/worker/worker.go`:
- Around line 168-172: The current block that calls w.enqueueRetry(ctx, msg,
err) (via retryScheduler) can drop the message if enqueueErr occurs; modify this
branch so that when enqueueErr != nil you perform a durable fallback before
returning: call the component that publishes to the DLQ (or reinsert into the
durable queue) with the original msg and error metadata (use the same message
payload and include err details), log any errors from that fallback, and only
then return; update the code around retryScheduler / enqueueRetry to invoke the
DLQ publish method (or durableQueue.Enqueue) as the fallback so messages are not
lost when retry enqueue fails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed7c5fad-5193-4381-b214-0075d44ab104
📒 Files selected for processing (7)
.env.examplecmd/issuetracker/main.gointernal/processor/validate/stage.gointernal/processor/validate/worker/worker.gointernal/processor/validate/worker/zset_intake.gotest/internal/processor/validate/worker/retry_job_test.gotest/internal/processor/validate/worker/zset_intake_test.go
gemini #3275211693 (Medium) 반영. 기존 30s 하드코딩 대신 msg.Headers["timeout_ms"] 헤더를 우선 사용 — republishForReparse 의 동일 정책과 통일. 변경: - BuildRetryJob: msg.Headers[core.HeaderTimeoutMs] 파싱하여 jobTimeout 결정, 부재 / 잘못된 값 시 buildRetryDefaultTimeout (30s) 사용 - buildRetryDefaultTimeout 상수 신설 — 의미 명확화 테스트 추가 (3 cases): - timeout_ms="60000" → 60s 사용 - 헤더 부재 → 30s default - 잘못된 값 (0/-1/non-numeric/empty) → 30s default Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…llback coderabbit 2건 Major 피드백 반영: #3275227525: VALIDATE_PRIORITY_QUEUE_ENABLED 활성화에 STAGES_VALIDATE_ENABLED 추가 의존: - 기존: stage 비활성 환경에서도 ZSET 분기 진입 → retryScheduler nil 시 fatal 가드 발동 - 변경: validatePriorityQueueEnabled = stagesCfg.ValidateEnabled && env && redisClientShared → Validate stage 가 disabled 면 ZSET wiring 자체 skip #3275227536: ZSET 모드에서 retry enqueue 실패 시 영구 손실 → DLQ fallback: - 기존: enqueueRetry 실패 시 단순 return → 메시지는 이미 ZSET 에서 pop, 영구 손실 - 변경: enqueueRetry 실패 시 sendToDLQ 호출 → 운영 가시성 + 수동 복구 가능 - DLQ 메시지의 reason 필드에 enqueue 에러 + 원본 process 에러 모두 보존 - DLQ 발행 성공 후 commit (ZSETConsumer 의 commit 은 no-op) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
pkg/queue.PriorityZSetQueue+PriorityPusher인터페이스 활용)구현 내용
메타 이슈 #515 의 Phase 2 Validate stage — Parser #522 와 동일 패턴 적용. Kafka partition FIFO 가 priority sub-ordering 을 제공 못 하는 한계를 Redis ZSET 으로 해소.
흐름:
Sub 1 — Validate Worker RetryScheduler hook (6c848ca)
Worker.retryScheduler bus.RetryScheduler필드 +SetRetrySchedulersetterHandle분기: process 실패 시enqueueRetry후 commit (메시지 손실 방지)BuildRetryJob(msg) (*core.CrawlJob, error): ProcessingMessage → ContentRef → CrawlJob 변환PriorityFromHeader헬퍼로 priority parsing 통일retry_reason="validate_process_failed"+original_ref_idmetadataMaxRetries = bus.DefaultMaxRetriesSub 2 — Kafka → ZSET intake goroutine (d301d92)
worker.ZSetIntake— Parser 동일 패턴, ContentRef 대응ContentRef.IDdefer consumer.Close()로 Kafka reader 자원 정리Sub 3 — Feature flag + main.go wiring + Stage 통합 (bc36508)
VALIDATE_PRIORITY_QUEUE_ENABLED=true+redisClientShared != nil시 ZSET 모드 활성validate.Stage.SetZSetIntake로 lifecycle 통합VALIDATE_ZSET_QUEUE_KEY/VALIDATE_ZSET_ENTRY_PREFIX/VALIDATE_ZSET_MAX_SIZE/VALIDATE_ZSET_ENTRY_TTL/VALIDATE_ZSET_POP_TIMEOUT.env.example갱신Sub 4 — 단위 테스트 (500a1c1)
CI / 머지 게이트 점검
변경 영향 범위
internal/processor/validate,cmd/issuetracker,.env.exampleRequired Status Checks
Commit LintPR Title LintLinked Issue CheckFormat CheckBuildTestLint롤백 계획
VALIDATE_PRIORITY_QUEUE_ENABLED=false(또는 unset) — 기존 Kafka consume 흐름 복귀후속 작업 (메타 #515 Phase 2)
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration
Tests