[REFAC#389] fetcher/worker/retry_scheduler → publisher.RetryScheduler 이동

[juhy0987]

연관 이슈

Closes #389
부모 메타: #385 — Publisher 통합 Sub 4

구현 내용

internal/processor/fetcher/worker/retry_scheduler.go 의 RetryScheduler 인프라를 internal/publisher/retry.go 로 통째로 이동. Kafka I/O 단일 책임 원칙 (이슈 #396) 에 따라 인터페이스도 publisher 측에서 정의.

이동 내역

Before (worker)	After (publisher)
worker.RetryScheduler interface	publisher.RetryScheduler
worker.retrySchedulerHolder (unexported)	publisher.RetrySchedulerHolder (exported S 필드 — atomic.Pointer 호환)
worker.KafkaImmediateRetryScheduler + NewKafkaImmediateRetryScheduler	publisher.*
worker.RedisDelayedRetryScheduler + NewRedisDelayedRetryScheduler	publisher.*
worker.RedisRetrySchedulerConfig + DefaultRedisRetrySchedulerConfig	publisher.*
worker.retryQueueClient (unexported)	publisher.retryQueueClient
worker.drainTimeout (재시도용 5s)	publisher.retryDrainTimeout (이름만 명시화)

worker.pool / manager 변경

• atomic.Pointer[retrySchedulerHolder] → atomic.Pointer[publisher.RetrySchedulerHolder]
• SetRetryScheduler(rs RetryScheduler) → SetRetryScheduler(rs publisher.RetryScheduler)
• resolveRetryScheduler() 의 fallback → publisher.NewKafkaImmediateRetryScheduler(p.producer)
• ManagerConfig.RetryScheduler 필드 타입 → publisher.RetryScheduler
• worker 측 topicForPriority / drainTimeout 은 유지 (requeueWithRetry 로그 + 다른 sendToDLQ/commit 경로에서 사용)

cmd/issuetracker/main.go wiring

• var retryScheduler crawlerWorker.RetryScheduler → publisher.RetryScheduler
• crawlerWorker.NewRedisDelayedRetryScheduler(...) → publisher.NewRedisDelayedRetryScheduler(...)
• crawlerWorker.DefaultRedisRetrySchedulerConfig() → publisher.DefaultRedisRetrySchedulerConfig()

테스트

• test/internal/processor/fetcher/worker/retry_scheduler_test.go 전체 → test/internal/publisher/retry_test.go 이동 (package publisher_test 로 재선언, worker.X → publisher.X 일괄 치환, mock producer 이름은 retryMockProducer 로 prefix 변경하여 다른 publisher_test 파일과 충돌 회피)
• pool.SetRetryScheduler 통합 시나리오 TestKafkaConsumerPool_SetRetryScheduler_BypassesInlinePublish 만 worker_test 측 신규 파일 pool_retry_scheduler_test.go 로 분리 — pool 의 mockConsumer/mockProducer/marshaledJobMsg/runPool 등 worker_test 헬퍼를 그대로 활용하면서 publisher.NewRedisDelayedRetryScheduler 로 inject

CI / 머지 게이트 점검

• gofmt -l — clean
• go build ./internal/... ./cmd/... ./pkg/... — pass
• go test -race -count=1 ./test/... — 전 패키지 통과 (publisher 1.41s / worker 1.22s 포함)
• PR 타이틀 [REFAC#389] 정규식 매칭
• commit 메시지 [REFAC]: prefix + 한국어

변경 영향 범위 + 위험도

• 영향: fetcher pool retry 경로, 메인 wiring, 테스트
• 위험도 Medium → 동작 동등성 확보됨 — retry scheduler 의 코드는 의미적으로 동일하게 이동. 인터페이스 노출 위치만 변경. 모든 기존 테스트가 새 위치에서 통과.
• pool 의 RetrySchedulerHolder.S 필드를 export 한 이유: atomic.Pointer 가 외부 패키지의 unexported 필드를 직접 다룰 수 없음. holder 자체가 wrapper 이므로 노출 영향 미미.

롤백 계획

• 본 PR revert 시: worker 측 retry_scheduler.go 파일이 git 히스토리에 그대로 있으므로 revert 시 즉시 복원. main.go 의 import 도 crawlerWorker 로 되돌아감.

후속

• Sub 5 ([REFACTOR] fetcher/worker Kafka Consumer + Publish 책임 분리 (#385 Sub 5) #390) — fetcher/worker Kafka Consumer + Publish 책임 분리
• Sub 6 ([REFACTOR] PriorityResolver chain → publisher 측 이동 + 모든 PublishX 통과 (#385 Sub 6) #391) — PriorityResolver chain publisher 이동
• Sub 7 ([REFACTOR] parser worker Kafka I/O → publisher facade (#385 Sub 7) #392) / Sub 8 ([REFACTOR] validate worker + cmd/processor Kafka I/O → publisher facade (#385 Sub 8) #393) — parser / validate Kafka I/O 통합

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

• Refactor
  • Reorganized retry scheduler architecture for improved internal consistency and message routing reliability.
  • Enhanced shutdown behavior for delayed retry queues.

[coderabbitai]

Warning

Rate limit exceeded

@juhy0987 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 50 minutes and 59 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4f538df5-fbc9-4a95-965e-db4a247d44ab

📥 Commits

Reviewing files that changed from the base of the PR and between f1f4085 and 6a3649c.

📒 Files selected for processing (7)

• internal/processor/fetcher/worker/manager.go
• internal/processor/fetcher/worker/pool.go
• internal/publisher/publisher.go
• internal/publisher/retry.go
• internal/scheduler/throttle.go
• test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go
• test/internal/publisher/retry_test.go

📝 Walkthrough

Walkthrough

The PR moves retry scheduler types and implementations from the worker package to the publisher package. Worker components (manager and pool) update their imports and type signatures to use publisher.RetryScheduler. Main command wiring and the comprehensive test suite are also migrated to reflect the new publisher-centric architecture.

Changes

Retry Scheduler Consolidation

Layer / File(s)	Summary
Retry Scheduler Foundation in Publisher internal/publisher/retry.go	Package migrated from worker to publisher; exports RetrySchedulerHolder for atomic swapping; updates immediate and delayed retry paths to use crawlTopic(...) instead of topicForPriority(...) for Kafka topic selection; introduces dedicated retryDrainTimeout for shutdown drain-context handling.
Worker Manager and Pool Type Updates internal/processor/fetcher/worker/manager.go, internal/processor/fetcher/worker/pool.go	Manager and pool import publisher and update RetryScheduler field type from local to publisher.RetryScheduler; pool's SetRetryScheduler method and resolveRetryScheduler function signature updated to accept and return publisher types; pool stores retry scheduler in atomic.Pointer[publisher.RetrySchedulerHolder].
Main Command Wiring Update cmd/issuetracker/main.go	Redis delayed retry scheduler construction switches from crawlerWorker.DefaultRedisRetrySchedulerConfig() to publisher.DefaultRedisRetrySchedulerConfig() and loads HeartbeatEveryNIdleTicks from config.LoadRetryScheduler().
Consumer Pool Retry Scheduler Test test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go	New test verifies that KafkaConsumerPool.SetRetryScheduler() accepts an injected publisher.RetryScheduler and routes retries through it, bypassing inline producer publish; uses in-memory poolRetryFakeQueue double.
Retry Scheduler Test Suite Migration test/internal/publisher/retry_test.go	Test suite moved from worker_test to publisher_test; updated mock producer (retryMockProducer) and all scheduler constructors to use publisher.NewKafkaImmediateRetryScheduler and publisher.NewRedisDelayedRetryScheduler; adjusted test doubles and assertions to validate enqueue/publish/ack semantics, heartbeat compression via HeartbeatEveryNIdleTicks, and drain-context behavior on shutdown.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• #389: Directly implements the retry scheduler migration sub-task, moving scheduler logic to publisher and updating worker component types.
• #390: Related at the code level—both refactor fetcher/worker to depend on publisher-provided interfaces and types for retry scheduling.
• #393: Directly related—both migrate retry-scheduling types and wiring to the publisher package as part of publisher facade consolidation.
• #385: Parent issue—this PR directly implements parts of the publisher consolidation objective of moving retry scheduler logic into the publisher package.

Suggested labels

refactor

Poem

🐰 A rabbit hops through packages with glee,
Shuffling retry schedulers to their rightful tree,
From worker to publisher, the types now align,
Atomic pointers swap, and the wiring shines fine—
One cohesive façade, oh what a sight! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title in Korean accurately summarizes the main change: moving retry_scheduler from fetcher/worker to publisher package. It's concise and clearly indicates the refactoring objective.
Linked Issues check	✅ Passed	The PR successfully fulfills all coding objectives from issue #389: moved RetryScheduler implementations to publisher package [#389], updated worker/manager and pool signatures to use publisher.RetryScheduler [#389], updated main.go wiring to publisher constructors [#389], and moved tests to test/internal/publisher [#389].
Out of Scope Changes check	✅ Passed	All changes are directly aligned with the stated objectives of issue #389. The file movements, type changes, test relocations, and wiring updates are all necessary components of the retry scheduler refactoring.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/#389/publisher-retry

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request refactors the RetryScheduler logic by moving it from the worker package to a new publisher package to adhere to the 'Kafka I/O single responsibility principle.' This involves updating type references across main.go, manager.go, and pool.go, as well as migrating and updating associated tests. Feedback includes improving the naming of the RetrySchedulerHolder field for better readability, centralizing topic mapping logic to avoid duplication, and cleaning up unused variables or restoring helpful comments in the test suite.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go (1)

56-56: ⚡ Quick win

Remove unused global counter.

The poolRetrySchedulerCounter is declared but never meaningfully used. Line 91 just loads it to silence the linter, which is a code smell.

Options:

1. If not needed, remove both the declaration and line 91
2. If needed for future tests, add a TODO comment explaining the intent

🤖 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 `@test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go` at line
56, Remove the unused global atomic counter by deleting the declaration
poolRetrySchedulerCounter and the only usage that calls
poolRetrySchedulerCounter.Load() (the linter-silencing read); if you intend to
keep it for future tests instead, add a clear TODO comment above
poolRetrySchedulerCounter explaining its intended purpose and why it is
currently unused so the linter-warning won't be silenced implicitly.

🤖 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/retry.go`:
- Line 73: The duplicate utility function crawlTopic is defined in both
internal/scheduler/throttle.go and internal/publisher/publisher.go; remove the
duplicate in publisher.go and consolidate a single crawlTopic implementation in
a shared location (e.g., keep it in internal/scheduler/throttle.go or create
internal/scheduler/priority.go), then update the publisher package to import and
call that shared crawlTopic function instead of its local copy; ensure the
consolidated function has the same signature and update imports and any package
references in publisher.go (e.g., calls in retry.go/publisher.go) so compilation
and behavior remain unchanged.

---

Nitpick comments:
In `@test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go`:
- Line 56: Remove the unused global atomic counter by deleting the declaration
poolRetrySchedulerCounter and the only usage that calls
poolRetrySchedulerCounter.Load() (the linter-silencing read); if you intend to
keep it for future tests instead, add a clear TODO comment above
poolRetrySchedulerCounter explaining its intended purpose and why it is
currently unused so the linter-warning won't be silenced implicitly.

🪄 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: 4df792bb-13ea-4711-b935-b0e0ef4cb7cb

📥 Commits

Reviewing files that changed from the base of the PR and between 3fe6259 and f1f4085.

📒 Files selected for processing (6)

• cmd/issuetracker/main.go
• internal/processor/fetcher/worker/manager.go
• internal/processor/fetcher/worker/pool.go
• internal/publisher/retry.go
• test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go
• test/internal/publisher/retry_test.go

[Copilot]

Pull request overview

fetcher/worker에 있던 RetryScheduler(즉시 Kafka 재발행 + Redis 지연 재시도)를 internal/publisher로 이동해, Kafka publish 책임을 publisher 패키지로 단일화하려는 리팩토링입니다(메타 #385의 “Kafka I/O 단일 책임” 방향에 맞춘 정리).

Changes:

• internal/publisher/retry.go로 RetryScheduler 인터페이스/구현체 및 Redis delayed retry 로직을 이동
• fetcher worker pool/manager가 publisher.RetryScheduler를 주입/해결하도록 타입 및 wiring 갱신
• main wiring 및 테스트를 신규 위치로 이동하고, pool 통합 시나리오 테스트를 worker_test에 분리 추가

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
internal/publisher/retry.go	RetryScheduler 인터페이스/구현체 및 Redis delayed retry 로직을 publisher로 이동, drain timeout 명시화
internal/processor/fetcher/worker/pool.go	retry scheduler holder를 publisher.RetrySchedulerHolder로 변경하고 fallback 생성 경로를 publisher로 연결
internal/processor/fetcher/worker/manager.go	ManagerConfig.RetryScheduler 타입을 publisher.RetryScheduler로 변경하여 주입 경로 일원화
cmd/issuetracker/main.go	RetryScheduler wiring을 worker → publisher 생성자로 전환
test/internal/publisher/retry_test.go	RetryScheduler 단위 테스트를 publisher_test로 이동하고 producer mock 충돌을 피하도록 정리
test/internal/processor/fetcher/worker/pool_retry_scheduler_test.go	pool.SetRetryScheduler 통합(인라인 publish 우회) 시나리오 테스트를 별도 파일로 분리 추가

Comments suppressed due to low confidence (1)

internal/publisher/retry.go:29

• RetrySchedulerHolder의 필드가 S로 export되어 있어, 외부 패키지가 holder 포인터를 보관한 뒤 S를 직접 변경할 수 있습니다. 이렇게 되면 atomic.Pointer로 교체한다는 의도와 달리 non-atomic 경로로 값이 바뀌거나(data race 위험) API가 불필요하게 노출될 수 있습니다. publisher 패키지에 holder 생성자(예: NewRetrySchedulerHolder) 또는 Set 메서드를 제공하고, 필드는 unexported로 유지하는 형태를 고려해 주세요.