[REFAC#197] crawler/worker 의 lock 인프라 → internal/locks 분리 (메타 이슈 #195 단계 2)

[juhy0987]

연관 이슈

• Closes [REFAC] crawler/worker 의 lock 인프라 → internal/locks 분리 (이슈 #195 단계 2) #197

구현 내용

메타 이슈 #195 의 단계 2 — internal/crawler/worker/ 의 단계 무관 lock 인프라를 internal/locks/ 패키지로 분리. fetcher/parser/validator 모두 동등한 위치에서 import 가능한 hub 위치.

코드 변경 (commit 1)

이동:

• internal/crawler/worker/ingestion_lock.go → internal/locks/ingestion_lock.go
• internal/crawler/worker/processing_lock.go → internal/locks/processing_lock.go
• 패키지명 worker → locks 로 변경

Caller import 갱신:

• cmd/issuetracker/main.go — crawlerWorker 유지 + locks 추가 (RetryScheduler 등 다른 심볼 때문에 둘 다 필요)
• cmd/processor/main.go — crawlerWorker 제거 + locks 추가 (lock symbol만 사용)
• internal/parser/worker/parser_worker.go — 동일 (crawlerWorker import 제거)
• internal/processor/validate/worker.go — 동일
• internal/crawler/worker/{manager,pool}.go — locks import 추가 (자기 패키지 내부에서 lock 사용)

테스트 mirror 분리:

• test/internal/worker/{ingestion,processing}_lock_test.go → test/internal/locks/
• processing_lock_test.go 의 단위 테스트 (NoopProcessingLock_*, ProcessingKey_Determinism) 만 새 위치 잔류
• pool 통합 테스트 3건 (TestKafkaConsumerPool_ProcessingLock_*) 은 worker 패키지 헬퍼 의존이라 test/internal/worker/processing_lock_pool_test.go 로 별도 신설
• 두 위치의 mockProcessingLock 은 각자 패키지 안에서만 사용

문서 변경 (commit 2)

• docs/architecture/internal/locks/README.md 신규 — 단일 패키지 doc
• docs/architecture/internal/crawler/worker.md — ProcessingLock / IngestionLock 섹션 제거 + locks 외부 링크 추가
• 다른 6개 doc (publisher.md / parser/README.md / processor/validate.md / pkg/redis.md / cmd/*.md / 최상 README.md) 의 lock 인용 위치 갱신
• .claude/rules/01-architecture.md 디렉토리 트리에 internal/locks/ 항목 추가

변경 후 구조

internal/
├── crawler/worker/         (PoolManager + KafkaConsumerPool + RetryScheduler + CircuitBreaker + PriorityResolver)
├── locks/                  ← 신규
│   ├── ingestion_lock.go   (IngestionLock + Redis/Noop)
│   └── processing_lock.go  (ProcessingLock + Redis/Noop + ProcessingKey + Stage{Fetcher,Parser,Validator})
├── parser/                 (parser engine + worker)
└── processor/validate/     (validate worker)

CI / 머지 게이트 점검

CI 운영 규약 및 Required Status Checks 단일 소스에 따라 작성합니다.

변경 영향 범위

• 영향 패키지/모듈: internal/crawler/worker/* (lock 부분 제거), internal/locks/* (신규), cmd/{issuetracker,processor}/main.go, internal/parser/worker/parser_worker.go, internal/processor/validate/worker.go, internal/crawler/worker/{manager,pool}.go (locks import), 다수 docs/architecture md
• 위험도(택1): Low-Medium
  • import 경로 + 패키지명만 변경 — Redis SETNX 로직 / 단계별 사용 패턴 / TTL 동작 변경 없음
  • git mv 로 git history 보존
  • go build ./... && go test -race ./... 28 패키지 모두 통과 (failure 0)
  • publisher 의 duck-typed IngestionLock interface 그대로 동작 (구조적 의존성 영향 0)

Required Status Checks

• 통과 확인 대상 (PR Checks 탭에서 확인):
  • Commit Lint
  • PR Title Lint
  • Linked Issue Check
  • Format Check
  • Build
  • Test
  • Lint

롤백 계획

• git revert 2개 commit (코드 / 문서). DB / Redis / Kafka 영향 없음.

TODO

• (다음 sub-issue) [REFAC] crawler 잔여 fetcher 영역 → processor/fetcher 이동 (이슈 #195 단계 3, 메타 이슈 close) #198 — internal/crawler/{core,handler,implementation,domain,rate_limiter,worker} → internal/processor/fetcher/... 이동 + 메타 이슈 [REFAC] crawler → processor 정렬 — 파이프라인 단계별 디렉토리 재배치 (메타 이슈) #195 close

논의 사항

• RetryScheduler 는 lock 과 함께 옮길지 검토했지만, 현재 fetcher worker 만 사용 + retry 발행 후의 priority topic 라우팅 책임이 있어 fetcher 영역에 잔류. 향후 parser/validator 도 retry queue 가 필요해지면 별도 sub-issue 로 단계 무관 위치로 이동 검토.
• CircuitBreaker / PriorityResolver 는 fetcher 전용이라 잔류.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Refactor

  • Reorganized internal distributed lock coordination into a dedicated module for improved code organization and separation of concerns across system stages.

• Tests

  • Updated test suites to align with refactored lock module structure and added deterministic behavior validation for lock key generation.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors the codebase to extract distributed lock infrastructure (IngestionLock and ProcessingLock) from internal/crawler/worker into a dedicated internal/locks package. The locks are moved as-is with only package declaration changes, and all callers (fetcher, parser, validator, publisher) and entry points are updated to import from the new location alongside corresponding architecture documentation.

Changes

Cohort / File(s)	Summary
Architecture Documentation - Lock Module docs/architecture/internal/locks/README.md	New documentation for the internal/locks package, defining IngestionLock and ProcessingLock interfaces, Redis implementations with SETNX/PX, noop variants for dev/single-process, stage constants, and ProcessingKey helper; documents usage patterns across ingestion and processing stages.
Architecture Documentation - Module Refactoring docs/architecture/README.md, docs/architecture/internal/crawler/worker.md, docs/architecture/pkg/redis.md	Updated to reflect lock infrastructure migration from crawler/worker to dedicated internal/locks; removed ProcessingLock responsibility from worker component summary and clarified RetryScheduler/CircuitBreaker as primary worker responsibilities.
Architecture Documentation - Component Dependencies docs/architecture/cmd/README.md, docs/architecture/cmd/issuetracker.md, docs/architecture/cmd/processor.md, docs/architecture/internal/parser/README.md, docs/architecture/internal/processor/validate.md, docs/architecture/internal/publisher.md	Updated dependency references to point ProcessingLock/IngestionLock to internal/locks package instead of internal/crawler/worker.
Lock Package Files internal/locks/processing_lock.go, internal/locks/ingestion_lock.go	Package declaration changed from worker to locks (moved from internal/crawler/worker/ directory); no logic changes.
Main Entry Points cmd/issuetracker/main.go, cmd/processor/main.go	Updated imports and type references for ProcessingLock, IngestionLock, and noop implementations to source from internal/locks package instead of internal/crawler/worker.
Crawler Worker - Type Migration internal/crawler/worker/manager.go, internal/crawler/worker/pool.go	Updated ManagerConfig.ProcessingLock field type and KafkaConsumerPool.procLock to use locks.ProcessingLock interface; lock key generation updated to use locks.ProcessingKey() and locks.StageFetcher constant.
Parser Worker - Type Migration internal/parser/worker/parser_worker.go	Migrated ParserWorker.procLock field and constructor parameter from crawlerWorker.ProcessingLock to locks.ProcessingLock; lock key generation updated to use locks.StageParser and locks.ProcessingKey().
Validator Worker - Type Migration internal/processor/validate/worker.go	Updated Worker struct field and NewWorker parameter to use locks.ProcessingLock and locks.NoopProcessingLock; lock key generation now uses locks.StageValidator and locks.ProcessingKey().
Test Updates - Lock Tests test/internal/locks/ingestion_lock_test.go, test/internal/locks/processing_lock_test.go	Updated ingestion lock tests to import from internal/locks package; new comprehensive test suite for processing lock covering noop behavior and ProcessingKey determinism.
Test Updates - Worker Tests test/internal/processor/validate/worker_test.go, test/internal/worker/processing_lock_pool_test.go	Updated to construct locks from internal/locks package; removed duplicate unit tests for NoopProcessingLock behavior and ProcessingKey in favor of centralized lock package tests.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related issues

• [REFAC] crawler → processor 정렬 — 파이프라인 단계별 디렉토리 재배치 (메타 이슈) #195: Parent meta-issue tracking the crawler/worker refactoring; this PR implements sub-issue #2 (lock infrastructure extraction from internal/crawler/worker to internal/locks).
• [REFAC] crawler 잔여 fetcher 영역 → processor/fetcher 이동 (이슈 #195 단계 3, 메타 이슈 close) #198: Depends on this PR as a prerequisite; the fetcher worker move requires locks to be already isolated in internal/locks for clean separation.

Possibly related PRs

• [FEAT#178] Ingestion Lock — URLCache 폐기 + Publisher 단일 책임 정규화 + atomic SETNX 진입 차단 #179: Introduces the original IngestionLock/ProcessingLock implementations and their Redis-backed variants; this PR reorganizes and re-homes those implementations into a dedicated package.
• [DOCS#193] docs/architecture/ — 모듈/연결 관계 트리 문서화 #194: Introduces initial architecture documentation for lock abstractions; this PR extends and updates that documentation to reflect the new internal/locks module structure.
• [FEAT#149] LLM 자동 parsing rule 생성 wiring (1차: cmd + llmgen + parser_worker fallback) #168: Modifies internal/parser/worker/parser_worker.go with additional dependencies; this PR updates the same file's lock type references as part of the package migration.

Suggested labels

refactor

Poem

🐰 Locks leap to their own cozy burrow,
From worker's crowded tunnels they borrow,
Now fetcher, parser, validator share,
A common home with organized care.
Stage-aware and neat—a rabbit's repair! 🔐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 56.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly specifies a lock infrastructure refactoring (#197) moving from crawler/worker to internal/locks, directly matching the main change described across all file summaries.
Linked Issues check	✅ Passed	All coding requirements from issue #197 are met: lock files moved via git mv, package renamed to locks, caller imports updated across all components (cmd, parser, validator, crawler internals), tests reorganized, documentation added, and CI checks passed.
Out of Scope Changes check	✅ Passed	All changes align with issue #197 scope: lock infrastructure extraction and reorganization. No unrelated fetcher worker migration, crawler core/handler movements, or out-of-scope refactoring is present.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/#197/locks-extraction

---

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

_{Review rate limit: 0/1 reviews remaining, refill in 60 minutes.}

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

[gemini-code-assist]

Code Review

This pull request refactors the distributed locking infrastructure by moving ProcessingLock and IngestionLock from the crawler worker package into a new, dedicated internal/locks package. This change promotes a stage-agnostic architecture, allowing the fetcher, parser, and validator components to share the same locking logic consistently. The refactor includes updates to all dependent services, comprehensive documentation changes, and the relocation of associated tests. I have no feedback to provide as there were no review comments to evaluate.

[coderabbitai]

🧹 Nitpick comments (1)

docs/architecture/internal/crawler/worker.md (1)

8-10: ⚡ Quick win

Align the external-systems section with the moved lock ownership.

Line 8 says lock infra moved to internal/locks, but the external-systems table still includes IngestionLock under crawler/worker scope, which is contradictory.

Suggested doc fix

-| Redis    | ProcessingLock (SETNX) / IngestionLock (SETNX) / RetryQueue (ZSET) |
+| Redis    | ProcessingLock (via internal/locks, SETNX) / RetryQueue (ZSET) |

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/architecture/internal/crawler/worker.md` around lines 8 - 10, Update the
external-systems section to reflect that lock ownership moved to internal/locks:
remove the IngestionLock entry from the crawler/worker scope (or change its
owner to internal/locks), and update the table row for locks to reference
ProcessingLock (used as locks.ProcessingLock(stage="fetcher", url)) under
internal/locks with a link to ../locks/README.md so the doc no longer
contradicts the earlier statement about the moved lock infra.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@docs/architecture/internal/crawler/worker.md`:
- Around line 8-10: Update the external-systems section to reflect that lock
ownership moved to internal/locks: remove the IngestionLock entry from the
crawler/worker scope (or change its owner to internal/locks), and update the
table row for locks to reference ProcessingLock (used as
locks.ProcessingLock(stage="fetcher", url)) under internal/locks with a link to
../locks/README.md so the doc no longer contradicts the earlier statement about
the moved lock infra.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e52b7880-7ba2-46d6-8887-445a009dfd93

📥 Commits

Reviewing files that changed from the base of the PR and between 137a6c8 and 7d4100c.

📒 Files selected for processing (23)

• .claude/rules/01-architecture.md
• cmd/issuetracker/main.go
• cmd/processor/main.go
• docs/architecture/README.md
• docs/architecture/cmd/README.md
• docs/architecture/cmd/issuetracker.md
• docs/architecture/cmd/processor.md
• docs/architecture/internal/crawler/worker.md
• docs/architecture/internal/locks/README.md
• docs/architecture/internal/parser/README.md
• docs/architecture/internal/processor/validate.md
• docs/architecture/internal/publisher.md
• docs/architecture/pkg/redis.md
• internal/crawler/worker/manager.go
• internal/crawler/worker/pool.go
• internal/locks/ingestion_lock.go
• internal/locks/processing_lock.go
• internal/parser/worker/parser_worker.go
• internal/processor/validate/worker.go
• test/internal/locks/ingestion_lock_test.go
• test/internal/locks/processing_lock_test.go
• test/internal/processor/validate/worker_test.go
• test/internal/worker/processing_lock_pool_test.go

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.