[REFAC#390] fetcher/worker Kafka I/O → publisher facade 위임 - #400
Conversation
메타 #385 Sub 5 — fetcher/worker 가 queue.Producer / queue.Consumer 를 직접 보유하지 않고 *publisher.Publisher 를 통해 Kafka I/O 를 수행하도록 변경. Kafka I/O 단일 책임 원칙 일관. publisher 추가: - type Consumer = queue.Consumer (별칭) — fetcher/worker 가 queue 패키지에 직접 의존하지 않도록 publisher 측 alias 노출 - (*Publisher).Forward(ctx, msg) — 호출자가 완성된 Message 를 publisher 내부 producer 로 그대로 발행하는 thin pass-through. publishX (PublishSeed/PublishUpgrade 등) 와 달리 호출자가 토픽/마샬링을 책임지는 경우의 escape hatch. publisher.retry 변경: - NewKafkaImmediateRetryScheduler(producer queue.Producer) → NewKafkaImmediateRetryScheduler(pub *Publisher) - NewRedisDelayedRetryScheduler 동일 변경 — 내부적으로 pub.Forward 사용 - 두 구현체 모두 producer 직접 호출 제거 worker.pool 변경: - 필드 producer queue.Producer → pub *publisher.Publisher - 필드 consumer queue.Consumer → consumer publisher.Consumer (별칭이라 호환) - 모든 NewKafkaConsumerPool* 생성자 시그니처 (producer → pub) - publishNormalized / sendToDLQ / resolveRetryScheduler 가 p.pub.Forward 또는 publisher.NewKafkaImmediateRetryScheduler(p.pub) 사용 worker.manager 변경: - PoolManager.producer queue.Producer → pub *publisher.Publisher - NewPoolManager 시그니처 변경 - Publish() 가 m.pub.Forward 사용 cmd/issuetracker/main.go wiring: - NewPoolManager(managerCfg, jobPublisher, ...) — 구 crawlerProducer 직접 주입 제거 - NewRedisDelayedRetryScheduler(redisClient, jobPublisher, ...) 동일 - crawlerProducer 는 sources.RegisterAll 등 비-worker 경로에서 계속 사용 (다음 sub 에서 정리 대상) 테스트: - newTestPublisher(producer queue.Producer) *publisher.Publisher 헬퍼 도입 (pool_test.go) — mockProducer 검증 흐름 유지 위해 thin wrapper - 동일 헬퍼를 publisher/retry_test.go 에 retryTestPub 으로 도입 - 모든 NewKafkaConsumerPool / WithCB / WithOptions 호출 시 producer → newTestPublisher(producer) 치환 - pool_retry_scheduler 의 SetRetryScheduler 시나리오도 같은 pub 인스턴스 공유 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 (4)
📝 WalkthroughWalkthroughFetcher worker pool, pool manager, and retry schedulers are refactored to depend on ChangesPublisher Facade Unification for Fetcher and Retry
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 refactors the Kafka I/O architecture by introducing a publisher.Publisher facade to decouple downstream modules, such as the fetcher and worker, from the queue package. It replaces direct queue.Producer dependencies with the new Publisher and introduces a Consumer type alias to centralize I/O responsibilities. Review feedback suggests completing this abstraction by also aliasing queue.Message, reducing code duplication by reusing internal message-building logic for retries, and further strengthening encapsulation by moving message construction logic from the worker manager into the Publisher facade.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@internal/publisher/publisher.go`:
- Around line 89-99: The Forward method can panic when called on a nil
*Publisher or when p.producer is nil; add explicit nil guards at the start of
Publisher.Forward to check if p == nil and if p.producer == nil and return a
descriptive error (e.g., using fmt.Errorf or errors.New) instead of calling
p.producer.Publish, preserving the existing signature and behavior for non-nil
cases and forwarding to p.producer.Publish only after the checks.
🪄 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: 65357a5e-265f-4523-b9b2-80410f34eb3c
📒 Files selected for processing (10)
cmd/issuetracker/main.gointernal/processor/fetcher/worker/manager.gointernal/processor/fetcher/worker/pool.gointernal/publisher/publisher.gointernal/publisher/retry.gotest/internal/processor/fetcher/worker/pool_gate_test.gotest/internal/processor/fetcher/worker/pool_retry_scheduler_test.gotest/internal/processor/fetcher/worker/pool_test.gotest/internal/processor/fetcher/worker/processing_lock_pool_test.gotest/internal/publisher/retry_test.go
There was a problem hiding this comment.
Pull request overview
fetcher/worker가 queue.Producer/queue.Consumer를 직접 보유하던 구조를 정리하고, *publisher.Publisher facade를 통해 Kafka publish 경로를 위임하도록 리팩토링한 PR입니다(#385 메타의 Kafka I/O 단일 책임 원칙을 fetcher/worker에도 적용).
Changes:
publisher.Consumer(=queue.Consumeralias)와(*Publisher).Forward(ctx, msg)escape hatch API를 추가해 다운스트림에서 producer 직접 보유를 제거publisher.retry(KafkaImmediate/RedisDelayed)의 publish 경로를producer.Publish→pub.Forward로 변경하고 시그니처를*Publisher주입 방식으로 통일- fetcher worker pool/manager 및 main wiring, 관련 테스트들을 새 facade 주입 형태로 일괄 갱신
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/publisher/publisher.go | Consumer alias 및 Forward pass-through API 추가 |
| internal/publisher/retry.go | RetryScheduler들이 queue.Producer 대신 *Publisher를 사용하도록 변경 |
| internal/processor/fetcher/worker/pool.go | pool이 producer 직접 보유 대신 pub.Forward로 publish 수행 |
| internal/processor/fetcher/worker/manager.go | manager가 producer 직접 보유 대신 pub.Forward로 publish 수행 |
| cmd/issuetracker/main.go | retry scheduler/manager wiring을 jobPublisher 주입으로 변경 |
| test/internal/publisher/retry_test.go | retry scheduler 테스트가 *Publisher 주입 형태로 변경되도록 헬퍼 추가/치환 |
| test/internal/processor/fetcher/worker/pool_test.go | worker pool 테스트에서 newTestPublisher 헬퍼로 publisher facade 주입 |
| test/internal/processor/fetcher/worker/processing_lock_pool_test.go | pool 생성 시 producer → publisher facade 주입으로 변경 |
| test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go | 커스텀 retry scheduler도 동일 pub 인스턴스 공유하도록 변경 |
| test/internal/processor/fetcher/worker/pool_gate_test.go | pool 생성 시 producer → publisher facade 주입으로 변경 |
PR #400 의 NewKafkaConsumerPool 시그니처 변경 (producer → publisher) 이 examples 트리도 빌드 대상이라는 점을 누락. 동일 패턴으로 publisher.New(producer, nil, log) wrap 적용. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
gemini + coderabbit 리뷰 반영:
1. publisher.Forward nil guard (coderabbit major)
- p == nil / p.producer == nil 시 panic 대신 명시적 error 반환
- retry/worker hot path 호출 보호 — silent crash 방지
2. publisher.Message type alias 추가 (gemini)
- type Message = queue.Message — Consumer 별칭과 대칭
- 다운스트림이 queue 패키지 직접 의존 없이 publisher.Message 만으로 Forward 호출 가능
- Forward 시그니처를 publisher.Message 로 표기 변경
3. publisher.PublishJob 신규 메소드 (gemini)
- CrawlJob 을 받아 buildMessage + Forward — manager.Publish 의 inline 로직 흡수
- manager.Publish 가 priority 결정 + 로깅만 담당 (marshal/topic/headers 중복 제거)
4. retry.go buildMessage 재사용 (gemini)
- KafkaImmediateRetryScheduler.Enqueue + RedisDelayedRetryScheduler.republish 모두
buildMessage 호출하여 crawler/priority 기본 헤더 부착 누락 해소
- retryHeaders 헬퍼 → applyRetryHeaders 로 변경 (덮어쓰기 in-place)
- Redis 경로의 Value 는 보관된 entry.JobBytes 유지 (재-marshal 회피)
5. Consumer 별칭 docstring 정정 (Copilot)
- 존재하지 않는 SubscribeCrawlTopic 언급 제거 — 현재 wiring 실제와 일치
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
Closes #390
부모 메타: #385 — Publisher 통합 Sub 5
구현 내용
fetcher/worker 가 queue.Producer / queue.Consumer 를 직접 보유하지 않고
*publisher.Publisher를 통해 Kafka I/O 를 수행하도록 책임 분리. 메타 #385 의 Kafka I/O 단일 책임 원칙 일관성 확보.publisher 패키지 신규 API
type Consumer = queue.Consumer(alias)(*Publisher).Forward(ctx, msg queue.Message) errorpublisher.retry 시그니처 변경
두 구현체 내부에서
s.producer.Publish→s.pub.Forward로 변경.worker.pool / worker.manager 변경
producer queue.Producer→pub *publisher.Publisherconsumer queue.Consumer→consumer publisher.Consumer(별칭이라 동일)NewKafkaConsumerPool/WithCB/WithOptions/NewPoolManagerpublishNormalized/sendToDLQ/resolveRetryScheduler/manager.Publish가 publisher facade 사용cmd/issuetracker/main.go wiring
crawlerProducer는sources.RegisterAll등 비-worker 경로에서 계속 사용 (해당 경로는 다음 sub 정리 범위).테스트
newTestPublisher(producer queue.Producer) *publisher.Publisher헬퍼 도입 (test/.../worker/pool_test.go) — 실제 publisher.New 로 mockProducer 를 wrap. mockProducer 의 Publish expectation 은 pub.Forward → producer.Publish 위임 경로로 그대로 트리거.retryTestPub헬퍼 추가 (test/internal/publisher/retry_test.go).worker.NewKafkaConsumerPool*호출 시 producer →newTestPublisher(producer)치환.TestKafkaConsumerPool_SetRetryScheduler_BypassesInlinePublish도 동일 pub 인스턴스 공유.CI / 머지 게이트 점검
gofmt -l— cleango build ./internal/... ./cmd/... ./pkg/... ./test/...— passgo test -race -count=1 ./test/internal/publisher/... ./test/internal/processor/fetcher/worker/... ./test/internal/scheduler/...— 전 패키지 통과[REFAC#390][REFAC]:prefix + 한국어변경 영향 범위 + 위험도
롤백 계획
PR revert 시 모든 시그니처가 동시에 원복 — wiring 회귀 없음.
🤖 Generated with Claude Code
Summary by CodeRabbit