[REFAC#285] 통합 PipelineGuard — Scheduler/Publisher 일관 적용 + Category 단명 TTL - #286
Conversation
…TL (이슈 #285) 라이브 검증 (2026-05-06, 1h 53m) 의 \`url already in pipeline\` 11,391건 노이즈 분석 결과: parser 가 카테고리 페이지에서 article URL 을 추출해 publish 할 때 24h IngestionLock 이 정상 차단하지만, publish 시도 자체 (정규화/Gate/Redis SETNX) 비용이 발생. 또한 시나리오 C (카테고리 URL 이 article 로 잘못 추출) 도 동일 lock 경로로만 차단됨. 본 PR 은 IngestionLock 의 적용 범위를 모든 publish 진입점으로 확장하고 Category 에 단명 TTL 을 도입 — Scheduler 의 정기 갱신 의도는 보존. 5가지 변경점: 1. internal/locks/ingestion_lock.go: IngestionLock 인터페이스에 AcquireWithTTL 추가 — 호출별 TTL override 지원. RedisIngestionLock / NoopIngestionLock 양쪽 구현. publisher 의 IngestionLock 인터페이스도 동기화 + fakeIngestionLock 갱신. 2. internal/locks/pipeline_guard.go (신규): IngestionLock 위에 target type 별 TTL 정책 적용. CheckAndAcquire 가 Category 면 categoryTTL (default 60s), 그 외는 IngestionLock default (24h). Release 메소드로 명시적 marker 제거. 3. pkg/config/config.go: PIPELINE_GUARD_CATEGORY_TTL 환경변수 (default 60s) 추가. RedisConfig.PipelineGuardCategoryTTL 필드. 4. internal/scheduler/emitter.go: JobEmitter 에 SetGuard + Emit 직전 CheckAndAcquire 추가. 가드 미주입 시 기존 동작 (fire-and-forget). 5. internal/publisher/publisher.go: SetPipelineGuard 추가. guard 우선, IngestionLock fallback (backward compat). Category 우회 제거 (guard 활성 시). acquireViaGuard 메소드 신설. 6. internal/processor/parser/worker/parser_worker.go: SetPipelineGuard + processCategoryPage defer 로 releaseCategoryMarker 호출. Category cycle (성공/handleRuleError/0 links/publish 실패) 모든 경로에서 release 보장. Release 실패는 non-fatal (TTL fallback). 7. cmd/issuetracker/main.go: pipelineGuard 구성 후 publisher / emitter / pw 모두 에 주입. ingestion 로그 메시지 갱신 (article_ttl + category_ttl 명시). 8. .env.example: PIPELINE_GUARD_CATEGORY_TTL 주석 + default 추가. 테스트 6건 (test/internal/locks/pipeline_guard_test.go): - Category 단명 TTL 적용 - Article default TTL 적용 - 중복 acquire false 반환 - Release 후 재진입 - nil lock fallback (Noop) - 0 TTL fallback to DefaultCategoryTTL 기대 효과 (라이브 검증 기준): - url already in pipeline 11,391건 → 거의 0 (publish 시도 전 차단) - 카테고리 cycle overlap 방지 - Scheduler 정기 갱신은 그대로 (TTL 60s 라 다음 2h 주기 진입 자연스러움) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits 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 (6)
📝 WalkthroughWalkthroughThis PR implements a unified ChangesUnified Pipeline Guard Implementation
Sequence DiagramsequenceDiagram
participant Scheduler as Scheduler<br/>(emit)
participant Guard as PipelineGuard
participant Lock as IngestionLock
participant Publisher as Publisher
participant Parser as Parser Worker
participant Fetcher as Fetcher Worker
Scheduler->>Guard: CheckAndAcquire(url, Category)<br/>targetType=Category
activate Guard
Guard->>Lock: AcquireWithTTL(url, 60s)<br/>short TTL for category
Lock-->>Guard: acquired=true
deactivate Guard
Guard-->>Scheduler: acquired=true
Scheduler->>Publisher: Emit → Kafka
Publisher->>Guard: CheckAndAcquire(url, Article)<br/>targetType=Article
Guard->>Lock: Acquire(url)<br/>uses default 24h TTL
Lock-->>Guard: acquired=true
Guard-->>Publisher: acquired=true
Publisher->>Publisher: publish message
Publisher-->>Fetcher: job → queue
Fetcher->>Fetcher: fetch & parse
Fetcher->>Parser: extracted links
Parser->>Guard: CheckAndAcquire(url, Article)<br/>for extracted article
Guard->>Lock: Acquire(url)
Lock-->>Guard: acquired=false<br/>(same URL in pipeline)
Guard-->>Parser: acquired=false
Parser-->>Parser: skip duplicate
Parser->>Parser: category cycle complete
Parser->>Guard: Release(url)
Guard->>Lock: Invalidate(url)
Lock-->>Guard: done
Note over Scheduler,Fetcher: Category marker released → next<br/>scheduler cycle can re-enter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Pull request overview
이 PR은 IngestionLock을 기반으로 한 통합 PipelineGuard를 도입해 Scheduler/Publisher/ParserWorker의 publish 진입점을 일관되게 보호하고, Category 타겟에 단명 TTL + 명시적 Release를 적용하여 “url already in pipeline” 노이즈 및 불필요한 publish 시도 비용을 줄이려는 리팩토링입니다.
Changes:
IngestionLock에AcquireWithTTL을 추가하고, 이를 래핑하는internal/locks.PipelineGuard를 신설해 Category(단명 TTL) / Article(기본 TTL) 정책을 통합- Scheduler emitter / Publisher / ParserWorker에 guard 주입 및 (Category) Release 연동
PIPELINE_GUARD_CATEGORY_TTL설정 추가 및 PipelineGuard 단위 테스트 추가
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/internal/publisher/publisher_ingestion_lock_test.go | publisher용 fake lock이 새 AcquireWithTTL 시그니처를 만족하도록 보강 |
| test/internal/locks/pipeline_guard_test.go | PipelineGuard의 Category TTL/Article TTL/중복 acquire/release/noop fallback 동작을 단위 테스트로 추가 |
| pkg/config/config.go | PIPELINE_GUARD_CATEGORY_TTL 설정을 RedisConfig에 추가하고 환경변수 파싱 로직 반영 |
| internal/scheduler/emitter.go | Emit 직전에 guard 체크를 수행하도록 추가 (중복 cycle 시 emit skip) |
| internal/publisher/publisher.go | PipelineGuard 우선 적용 + IngestionLock fallback 구조로 publish 진입 dedup 로직 확장 |
| internal/processor/parser/worker/parser_worker.go | Category 처리 종료 시 guard Release를 defer로 보장하도록 추가 |
| internal/locks/pipeline_guard.go | (신규) target type별 TTL 정책을 적용하는 PipelineGuard 구현 추가 |
| internal/locks/ingestion_lock.go | AcquireWithTTL을 인터페이스/구현체(Redis/Noop)에 추가 |
| cmd/issuetracker/main.go | PipelineGuard 구성 및 publisher/emitter/parser worker에 주입 wiring |
| .env.example | PIPELINE_GUARD_CATEGORY_TTL 예시/주석 추가 |
There was a problem hiding this comment.
Code Review
This pull request introduces PipelineGuard, a unified locking mechanism that applies target-specific TTL policies—such as short-lived locks for Categories and 24-hour locks for Articles—across the publisher, scheduler, and parser worker. Key updates include the addition of AcquireWithTTL to the ingestion lock interface and the implementation of explicit lock release in the parser worker for category cycles. Feedback highlights a potential issue with missing URL normalization in the scheduler's emitter, which could lead to inconsistent lock keys compared to the publisher, and suggests using a switch statement in the guard logic to improve extensibility and safety for future target types.
Copilot 리뷰 3건 반영: 1. (scheduler) emitter.Emit 이 guard skip 시 nil 반환하면 scheduler.publish 가 "crawl job scheduled" Info 로 기록 → 실제 발행 안 된 job 이 발행된 것처럼 misleading. ErrEmitSkipped sentinel 도입 — emitter 가 skip 시 본 에러 반환, scheduler 가 errors.Is 로 분기하여 "scheduled" / "failed" 로그 모두 생략. 2. (parser_worker) PipelineGuard 인터페이스 주석이 "internal/locks 를 직접 import 하지 않도록" 으로 되어 있으나 실제 파일은 locks 를 import 함. 의도 (Release 만 필요한 최소 인터페이스 분리) 반영하여 주석 정정. 3. (publisher) acquireIngestion 의 GoDoc 블록이 acquireViaGuard 함수 위에 붙어 GoDoc 가 잘못 매핑됨. 주석을 각각 자기 함수 선언 바로 위로 분리. acquireIngestion 에는 Deprecated 표시 추가 (guard 우선 정책 명시). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/publisher/publisher.go (1)
64-71:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStruct doc comment is stale after this PR.
Line 69 states
TargetTypeCategory 는 lock 미적용but whenPipelineGuardis active (the new primary path), Category URLs are acquired viaCheckAndAcquirewith the short-lived TTL. The "no lock for Category" wording now only applies to the legacyIngestionLockfallback path (line 194).✏️ Suggested doc update
- // - TargetTypeCategory 는 lock 미적용 (카테고리 페이지는 매 주기 새 기사 추출이 목적) + // - TargetTypeCategory 는 PipelineGuard 경로에서 단명 TTL (default 60s) 적용; + // IngestionLock fallback 경로에서만 lock 미적용 (backward compat)🤖 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/publisher/publisher.go` around lines 64 - 71, Update the struct doc comment to reflect current behavior: change the sentence that says "TargetTypeCategory 는 lock 미적용" to clarify that when PipelineGuard is active Category URLs are acquired via PipelineGuard.CheckAndAcquire with a short-lived TTL, and that "no lock for Category" only applies to the legacy IngestionLock fallback path (IngestionLock and its behavior at the old fallback code path remains unchanged); mention that lock lookup failures remain fail-open. Locate references to PipelineGuard, CheckAndAcquire, and IngestionLock in the comment and replace the stale statement with this concise, accurate description.
🤖 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 41-44: The publisher.IngestionLock interface has an unused method
AcquireWithTTL which forces mocks/implementations (e.g., fakeIngestionLock,
SetIngestionLock users) to implement an unnecessary breaking change; remove
AcquireWithTTL from the publisher.IngestionLock interface, update any
tests/mocks that implemented it, and drop the now-unused time import from
internal/publisher/publisher.go; keep behavior that acquireIngestion and callers
(which use lock.Acquire and PipelineGuard.CheckAndAcquire /
internal/locks.IngestionLock) remain unchanged.
In `@internal/scheduler/emitter.go`:
- Around line 47-64: When Emit acquires a guard via e.guard.CheckAndAcquire but
producer.Publish later fails, the guard marker is never released causing
CheckAndAcquire to block retries; add a Release(ctx, url, targetType) method to
the PipelineGuard interface (locks.PipelineGuard already implements Release)
and, inside Emit after a failed producer.Publish call, call e.guard.Release(ctx,
job.Target.URL, job.Target.Type) before returning the publish error so the guard
is cleared and scheduler retries can proceed.
---
Outside diff comments:
In `@internal/publisher/publisher.go`:
- Around line 64-71: Update the struct doc comment to reflect current behavior:
change the sentence that says "TargetTypeCategory 는 lock 미적용" to clarify that
when PipelineGuard is active Category URLs are acquired via
PipelineGuard.CheckAndAcquire with a short-lived TTL, and that "no lock for
Category" only applies to the legacy IngestionLock fallback path (IngestionLock
and its behavior at the old fallback code path remains unchanged); mention that
lock lookup failures remain fail-open. Locate references to PipelineGuard,
CheckAndAcquire, and IngestionLock in the comment and replace the stale
statement with this concise, accurate description.
🪄 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: f0689c41-6330-4c31-b268-c032d7d1a7be
📒 Files selected for processing (10)
.env.examplecmd/issuetracker/main.gointernal/locks/ingestion_lock.gointernal/locks/pipeline_guard.gointernal/processor/parser/worker/parser_worker.gointernal/publisher/publisher.gointernal/scheduler/emitter.gopkg/config/config.gotest/internal/locks/pipeline_guard_test.gotest/internal/publisher/publisher_ingestion_lock_test.go
gemini-code-assist 리뷰 2건 반영: 1. (emitter) job.Target.URL 을 정규화 없이 guard 에 전달 — publisher 는 normalize 후 CheckAndAcquire 호출. seed URL 이 정규형 아니면 marker 키 불일치 → 동일 logical URL 이 두 입구 (scheduler / publisher) 에서 다른 Redis 키 사용. JobEmitter.SetNormalizer 추가, Emit 시 guard 호출 직전 정규화 (정규화 실패는 fail-open). main.go 에서 publisher 와 같은 links.NewNormalizer 주입. 2. (PipelineGuard.CheckAndAcquire) Category 외 모든 target type 이 default Acquire (24h) 로 fall-through — 향후 Sitemap 등 단명 lock 필요한 type 도입 시 의도치 않은 24h TTL 적용 위험. switch case 로 명시적 분기 (Category / Article / default) — 새 type 추가 시 명시 case 추가 권장 주석. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… release CodeRabbit 리뷰 2건 반영: 1. (Major #3198418223) publisher.IngestionLock 의 AcquireWithTTL 이 dead surface. acquireIngestion 은 lock.Acquire 만 호출, TTL 경로는 internal/locks 의 IngestionLock 을 PipelineGuard.CheckAndAcquire 로 사용. publisher-local 인터페이스에 AcquireWithTTL 두는 것은 ISP 위반 + 테스트 mock 강제 변경 유발. publisher.IngestionLock 에서 AcquireWithTTL 제거. fakeIngestionLock 의 dummy 구현도 제거. 2. (Minor #3198418228) emitter 가 CheckAndAcquire 로 marker 를 잡은 후 producer.Publish 가 실패하면 marker 가 TTL (Category 60s) 까지 점유 — 다음 retry 가 (false, nil) 받아 ErrEmitSkipped 로 silent skip → job 손실. PipelineGuard 인터페이스에 Release 추가. publish/marshal 실패 시 releaseGuardOnFailure 헬퍼로 marker 즉시 해제. Release 실패는 non-fatal (TTL fallback 으로 자연 회수). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CodeRabbit outside diff range 리뷰 반영: Publisher 구조체의 dedup 정책 주석이 PR 변경 후 stale 상태. \"TargetTypeCategory 는 lock 미적용\" 문구가 PipelineGuard 활성 시에는 사실과 다름 — Category URL 도 단명 TTL 로 marker 잡음. 주석을 현재 동작에 맞춰 갱신: - SetPipelineGuard 우선 — 모든 target type 적용 (Article 24h / Category 단명) - SetIngestionLock fallback — Article 만 (Category 우회는 legacy 경로 한정) - 둘 다 미설정 시 dedup 비활성 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
구현 내용
라이브 검증 (2026-05-06, 1h 53m) 의
url already in pipeline11,391건 노이즈 분석 결과, parser 가 카테고리 페이지에서 article URL 을 추출해 publish 할 때 24h IngestionLock 이 정상 차단하지만 publish 시도 자체 (정규화 / Gate / Redis SETNX) 비용이 발생. 또한 시나리오 C (카테고리 URL 이 article 로 잘못 추출) 도 동일 경로로만 차단.본 PR 은 IngestionLock 의 적용 범위를 모든 publish 진입점으로 확장 + Category 에 단명 TTL 도입 — Scheduler 의 정기 갱신 의도는 보존.
변경점 8가지
internal/locks/ingestion_lock.goIngestionLock인터페이스에AcquireWithTTL추가 (호출별 TTL override). RedisIngestionLock / NoopIngestionLock 양쪽 구현internal/locks/pipeline_guard.go(신규)PipelineGuard신설. Category=단명 TTL / Article=default TTL.Release메소드pkg/config/config.goPIPELINE_GUARD_CATEGORY_TTL환경변수 (default 60s)internal/scheduler/emitter.goJobEmitter.SetGuard+ Emit 직전 CheckAndAcquireinternal/publisher/publisher.goSetPipelineGuard추가. guard 우선, IngestionLock fallback (backward compat). Category 우회 제거internal/processor/parser/worker/parser_worker.goSetPipelineGuard+ processCategoryPage defer 로 release 호출cmd/issuetracker/main.go.env.examplePIPELINE_GUARD_CATEGORY_TTL주석 + default테스트 6건 (
test/internal/locks/pipeline_guard_test.go)기대 효과 (라이브 검증 기준)
url already in pipelineDebug 로그CI / 머지 게이트 점검
변경 영향 범위
internal/locks,internal/scheduler,internal/publisher,internal/processor/parser/worker,pkg/config,cmd/issuetrackerMediumRequired Status Checks
Commit LintPR Title LintLinked Issue CheckBuildTestLint로컬 검증
make fmt통과make build5개 binary 모두 통과go test -race ./...전체 통과 (신규 6건 포함)go vet ./...통과롤백 계획
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Configuration
PIPELINE_GUARD_CATEGORY_TTLenvironment variable (default: 60s) for configuring category processing behaviorTests