[FEAT#510] normal/low crawl 토픽 Redis 버퍼링 + 주기적 Kafka backlog drain - #511
Conversation
- pkg/redis/job_buffer.go: EnqueueJob / DrainJobs / JobBufferLen - LIST 자료구조: LPUSH 로 head enqueue, RPOP COUNT 로 tail FIFO drain (Redis 6.2+) - MaxLen 전달 시 LTRIM 으로 oldest 제거 (buffer 무한 누적 방어) - 기존 retry_queue.go (ZSET) 와 동일 패키지 컨벤션 — Client 메소드 + JobBufferKey 헬퍼 - 단위 테스트 5개 (FIFO / MaxLen LTRIM / empty / validation / Len) — Redis 환경에서 통합 검증 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…510) - pkg/queue/buffering_producer.go: queue.Producer 데코레이터 - topic == TopicCrawlNormal/Low → JobBuffer.EnqueueJob - 그 외 (high / raw / normalized / ...) → underlying.Publish 직접 - buffer enqueue 실패 시 underlying 으로 fallback — Redis 장애가 publish 자체 차단하지 않도록 - PublishBatch 도 메시지 단위 routing (mixed batch 지원) - pkg/queue/buffered_message.go: BufferedAt 기록 포함 JSON 직렬화 + DecodeBufferedMessage (drainer 사용) - NoopJobBuffer / Underlying() 접근자 노출 — drainer 무한 루프 회피용 - 단위 테스트 9개 (mock JobBuffer + Producer) — normal/low/high routing + fallback + nil buffer + Encode/Decode Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- internal/scheduler/buffer_drainer.go: 별도 goroutine - 매 tick (default 30s) 마다 normal/low 각 priority 별로: - queue.BacklogChecker.Backlog 로 현재 lag 조회 - available = TargetBacklog - lag (음수면 skip) - n := min(available, DrainBatch, JobBuffer.JobBufferLen) - JobBuffer.DrainJobs → underlying.PublishBatch - publish 실패 시 drained payload 재적재 (best-effort, 순서 보존 X) - Start/Stop graceful shutdown 지원 — 진행 중 cycle 완료 대기 - 부팅 직후 1회 즉시 drain — 이전 세션 잔존물 회복 - 단위 테스트 5개 (mock buffer + producer + checker): drain available / skip on threshold / re-enqueue on fail / idle / nil deps validation Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- pkg/config/processor/job_buffer.go: PUBLISHER_REDIS_BUFFER_* - ENABLED (default false — opt-in, backward compatible) - DRAIN_INTERVAL (30s) / TARGET_BACKLOG (3000) / DRAIN_BATCH (100) - MAX_LEN (100000 — LTRIM cap) / CHECK_TIMEOUT (5s) - pkg/config/internal/parse: PositiveInt64 helper 추가 (TargetBacklog 검증용) - .env.example: 신규 env 6개 + 운영 가이드 주석 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…510) - Redis 클라이언트 초기화를 crawlerProducer 생성 전으로 이동 — JobBuffer / ProcessingLock / IngestionLock / RetryScheduler 가 공유 - LoadJobBuffer + bufferCfg.Enabled && redisClient != nil 시 wiring: - rawCrawlerProducer (KafkaProducer) 를 BufferingProducer 로 감싸 jobPublisher 에 주입 - BufferDrainer 는 rawCrawlerProducer (underlying) 로 직접 publish — 무한 루프 회피 - 활성화 로그에 drain_interval / target_backlog / drain_batch / max_len 출력 - 기능 비활성 (default) 시 raw producer 직접 사용 — 기존 동작 100% 보존 - Redis 미연결 시 자동 비활성 + WARN — graceful degrade - shutdown 체인: sched.Stop() 직후 bufferDrainer.Stop() — scheduler 의 마지막 publish 가 buffer 에 들어간 뒤 drainer 가 비울 기회 확보 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 (8)
📝 WalkthroughWalkthroughThis PR implements Redis-backed buffering for normal/low-priority crawl job publishing, with a periodic drainer that publishes buffered messages back to Kafka when consumer backlog is below a configured threshold. The feature is opt-in via environment variables and gracefully degrades to direct publishing when Redis is unavailable or buffering is disabled. ChangesRedis Job Buffering & Periodic Draining
Sequence DiagramssequenceDiagram
participant Scheduler
participant BufferingProducer
participant RedisBuffer
participant KafkaProducer
Scheduler->>BufferingProducer: Publish(message)
alt Normal or Low Priority
BufferingProducer->>BufferingProducer: encodeBufferedMessage()
BufferingProducer->>RedisBuffer: EnqueueJob(topic, payload, maxLen)
RedisBuffer-->>BufferingProducer: success or error
alt Enqueue Fails
BufferingProducer->>KafkaProducer: Publish(direct fallback)
end
else High Priority or Non-Crawl Topic
BufferingProducer->>KafkaProducer: Publish(direct)
end
KafkaProducer-->>BufferingProducer: success or error
sequenceDiagram
participant BufferDrainer
participant BacklogChecker
participant RedisBuffer
participant KafkaProducer
loop Every DrainInterval
BufferDrainer->>BacklogChecker: Backlog(groupID)
BacklogChecker-->>BufferDrainer: currentLag
alt Lag >= TargetBacklog
BufferDrainer->>BufferDrainer: Skip drain (throttled)
else Lag < TargetBacklog
BufferDrainer->>RedisBuffer: DrainJobs(priority, available)
RedisBuffer-->>BufferDrainer: payloads
BufferDrainer->>BufferDrainer: DecodeBufferedMessage(payloads)
BufferDrainer->>KafkaProducer: PublishBatch(messages)
alt Publish Success
BufferDrainer->>BufferDrainer: Log drain count
else Publish Failure
BufferDrainer->>RedisBuffer: EnqueueJob(failed payloads)
RedisBuffer-->>BufferDrainer: re-enqueued
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)
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 Redis-based buffering mechanism for normal and low priority crawl messages to prevent Kafka overloading. Key additions include a BufferingProducer for message routing, a BufferDrainer for controlled publishing based on consumer lag, and the necessary configuration and storage logic. The reviewer recommends enhancing the JobBuffer interface and its Redis implementation with batch enqueueing methods to reduce network overhead and improve performance during bulk operations.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
test/internal/scheduler/buffer_drainer_test.go (1)
176-178: ⚡ Quick winReplace fixed sleeps with eventual assertions to reduce flakiness.
These tests synchronize async work via fixed delays, which can intermittently fail in slower CI environments. Prefer
require.Eventually/assert.Eventuallywith bounded timeout + polling interval.Example pattern
d.Start(ctx) - time.Sleep(80 * time.Millisecond) + require.Eventually(t, func() bool { + return prod.count() == 5 + }, time.Second, 10*time.Millisecond) cancel() d.Stop() assert.Equal(t, 5, prod.count(), "buffer 5건 모두 drain 후 underlying publish")Also applies to: 205-207, 237-239, 263-265
🤖 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/scheduler/buffer_drainer_test.go` around lines 176 - 178, Replace the fragile fixed time.Sleep calls in the buffer_drainer tests with eventual assertions (require.Eventually or assert.Eventually) that poll until the expected condition is true within a bounded timeout; for each occurrence around the time.Sleep + cancel() + d.Stop() pattern, assert the async work completed (e.g., buffer length becomes 0, goroutine has exited, or a flag is set) before calling cancel() and d.Stop(), using a short polling interval and a generous timeout to avoid CI flakiness and apply the same change to the other similar blocks.
🤖 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/scheduler/buffer_drainer.go`:
- Around line 237-246: The recovery loop uses the potentially cancelled ctx when
calling d.buffer.EnqueueJob, risking data loss if shutdown cancelled ctx; change
it to create and use a non-cancelled bounded context (e.g., context.Background()
or context.WithTimeout) for the re-enqueue calls so EnqueueJob runs even if the
original ctx is cancelled. Update the code around d.producer.PublishBatch and
the subsequent for loop that calls d.buffer.EnqueueJob to derive and pass the
new recoveryCtx (with a small timeout) instead of the original ctx, and ensure
recoveryCtx is cancelled after use.
In `@pkg/queue/buffering_producer.go`:
- Around line 72-81: NewBufferingProducer currently accepts nil for required
dependencies and stores them, which leads to nil-pointer panics later; update
NewBufferingProducer to validate inputs and fail fast: if underlying == nil or
log == nil, return an immediate panic (or explicit runtime error) with a clear
message like "NewBufferingProducer: underlying Producer is required" /
"NewBufferingProducer: log is required" so callers get a clear error at
construction time; keep the existing fallback to NoopJobBuffer for buffer but do
not allow nil underlying or nil log to be stored on the BufferingProducer struct
(referencing NewBufferingProducer, BufferingProducer.underlying and
BufferingProducer.log which are used by methods that assume non-nil).
---
Nitpick comments:
In `@test/internal/scheduler/buffer_drainer_test.go`:
- Around line 176-178: Replace the fragile fixed time.Sleep calls in the
buffer_drainer tests with eventual assertions (require.Eventually or
assert.Eventually) that poll until the expected condition is true within a
bounded timeout; for each occurrence around the time.Sleep + cancel() + d.Stop()
pattern, assert the async work completed (e.g., buffer length becomes 0,
goroutine has exited, or a flag is set) before calling cancel() and d.Stop(),
using a short polling interval and a generous timeout to avoid CI flakiness and
apply the same change to the other similar blocks.
🪄 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: e73cdee4-2dc6-4112-9923-d92da6eaa73a
📒 Files selected for processing (11)
.env.examplecmd/issuetracker/main.gointernal/scheduler/buffer_drainer.gopkg/config/internal/parse/parse.gopkg/config/processor/job_buffer.gopkg/queue/buffered_message.gopkg/queue/buffering_producer.gopkg/redis/job_buffer.gotest/internal/scheduler/buffer_drainer_test.gotest/pkg/queue/buffering_producer_test.gotest/pkg/redis/job_buffer_test.go
…T (gemini) - JobBuffer interface 에 EnqueueBatch(label, payloads, maxLen) 추가 - pkg/redis/Client.EnqueueBatch: LPush variadic args + optional LTRIM 을 단일 pipeline 으로 전송 - EnqueueJob 을 EnqueueBatch 로 delegate 리팩터링 — single source of truth - NoopJobBuffer.EnqueueBatch 도 동등 error fallback - BufferingProducer.PublishBatch: label 별 그룹핑 후 1회 EnqueueBatch — 같은 label 의 N개 메시지 1 RTT - BufferDrainer publish 실패 re-enqueue 경로도 EnqueueBatch 사용 — 실패 복구 시 round-trip 절감 - 단위 테스트 추가: Redis 통합 4개 (batch / empty / LTRIM / invalid payload) + BufferingProducer label-별 단일 호출 검증 1개 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
normal/low priority crawl 토픽의 Kafka backlog 급증 시 publish 손실을 줄이기 위해 Redis LIST 기반 버퍼와 주기적 drainer 를 추가하는 PR입니다.
Changes:
- Redis JobBuffer, BufferingProducer, buffered message 직렬화 추가
- BufferDrainer 및 main wiring/env 설정 추가
- Redis/queue/drainer 단위 테스트와
.env.example갱신
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/redis/job_buffer.go |
Redis LIST 기반 enqueue/drain/len 구현 |
pkg/queue/buffering_producer.go |
normal/low topic 을 Redis buffer 로 라우팅하는 Producer 데코레이터 추가 |
pkg/queue/buffered_message.go |
Redis 저장용 queue message JSON 직렬화/역직렬화 추가 |
internal/scheduler/buffer_drainer.go |
backlog 기준 Redis buffer → Kafka drain goroutine 추가 |
pkg/config/processor/job_buffer.go |
JobBuffer env 설정 로더 추가 |
pkg/config/internal/parse/parse.go |
PositiveInt64 env parse helper 추가 |
cmd/issuetracker/main.go |
Redis 공유 클라이언트 조기 초기화, buffer/drainer wiring 및 shutdown 추가 |
.env.example |
Redis buffer 관련 환경 변수 문서화 |
test/pkg/redis/job_buffer_test.go |
Redis JobBuffer 동작 테스트 추가 |
test/pkg/queue/buffering_producer_test.go |
BufferingProducer 라우팅/직렬화 테스트 추가 |
test/internal/scheduler/buffer_drainer_test.go |
BufferDrainer drain/failure 경로 테스트 추가 |
Comments suppressed due to low confidence (2)
internal/scheduler/buffer_drainer.go:215
- 여기서
DrainJobs가 Redis LIST 에서 항목을 먼저 제거한 뒤 Kafka publish 를 시도하므로, 프로세스가 pop 이후 publish/재적재 전에 종료되면 해당 job 은 Redis 와 Kafka 양쪽에서 사라집니다. 같은 코드베이스의 retry queue 는 peek-publish-ack 패턴으로 crash 후 재처리를 보장하므로(pkg/redis/retry_queue.go:68-83), 이 버퍼도 processing list/ack 또는 peek 후 성공 시 제거 방식으로 바꾸지 않으면 "drop 해소" 목적과 달리 새로운 데이터 손실 창이 생깁니다.
// 3) drain
payloads, err := d.buffer.DrainJobs(ctx, label, n)
internal/scheduler/buffer_drainer.go:176
drainOnce가 backlog 조회, drain 수 계산, Redis pop, decode, Kafka publish, 실패 재적재까지 한 함수에 모두 들어 있어 변경 범위가 커지고 오류 경로 추적이 어렵습니다. 특히 손실 방지 로직은 별도 helper 로 분리해 성공/실패 경계를 명확히 하는 편이 유지보수와 테스트에 안전합니다.
func (d *BufferDrainer) drainOnce(ctx context.Context, label, topic string) error {
// 1) 현재 backlog 조회
checkCtx := ctx
if d.checkTimeout > 0 {
var cancel context.CancelFunc
checkCtx, cancel = context.WithTimeout(ctx, d.checkTimeout)
…afe 외 (coderabbit + Copilot) - (#511 핵심 결함, Copilot) BufferingProducer 활성 시 scheduler BacklogThrottler wiring 자체 skip - 기존 throttler 는 publish 직전 normal/low job drop → BufferingProducer 가 routing 받지도 못함 - bufferDrainer != nil 시 throttle 무시 + INFO 로그 'publisher redis buffer manages elastic queueing' - buffer 가 elastic queueing 책임 인수 — 본 PR 의 핵심 목적 달성 보장 - (coderabbit major) BufferDrainer re-enqueue 경로에 context.WithoutCancel + 5s timeout - shutdown 시 ctx cancel 됐어도 재적재 시도 → 데이터 손실 회피 - (coderabbit major + Copilot) BufferingProducer 생성자 nil guard (underlying / log 필수) - (Copilot) NoopJobBuffer doc 정리 — EnqueueJob/Batch 가 error 반환하는 의도 명시 - (Copilot) TestJobBufferLen t.Cleanup 추가 — 잔존 buffer 가 후속 테스트 영향 회피 - (Copilot) BufferDrainer low priority 테스트 추가 — drainTargets 의 low 매핑 회귀 catch - (Copilot) LoadJobBuffer env 테스트 신규 (default / override / invalid 8 케이스) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
연관 이슈
구현 내용
PR #509 머지 후 15분 라이브에서 관측된
kafka backlog exceeds threshold, throttling publish33건 drop 해소를 위한 Redis 버퍼 + 주기적 drainer 도입. high priority 는 영향 없음.구조
신규 컴포넌트 (5 commits)
pkg/redis/job_buffer.go— Redis LIST 기반 EnqueueJob/DrainJobs/JobBufferLenretry_queue.go(ZSET) 와 같은pkg/redis/Client메소드 패턴pkg/queue/buffering_producer.go+buffered_message.go— 데코레이터 + 직렬화BufferingProducer: topic 기반 routing, fallback 안전망, batch 지원bufferedMessageJSON 직렬화 (BufferedAt 포함 — drainer 의 체류 시간 metric 용)NoopJobBuffer/Underlying()노출 — wiring/drainer 안전망internal/scheduler/buffer_drainer.go— goroutinepkg/config/processor/job_buffer.go— 6개 env +parse.PositiveInt64신규 helpercmd/issuetracker/main.go— Redis 클라이언트 위쪽으로 이동 + wiring + shutdown 체인환경 변수 (
.env.example갱신)PUBLISHER_REDIS_BUFFER_ENABLEDfalsePUBLISHER_REDIS_BUFFER_DRAIN_INTERVAL30sPUBLISHER_REDIS_BUFFER_TARGET_BACKLOG3000SCHEDULER_MAX_BACKLOG5000 의 60%)PUBLISHER_REDIS_BUFFER_DRAIN_BATCH100PUBLISHER_REDIS_BUFFER_MAX_LEN100000PUBLISHER_REDIS_BUFFER_CHECK_TIMEOUT5s단위 테스트 (19개)
test/pkg/redis/job_buffer_test.go(5): FIFO / MaxLen LTRIM / empty / validation / Len — Redis 환경에서 통합 검증 (NOAUTH 시 skip)test/pkg/queue/buffering_producer_test.go(9): normal/low/high routing / non-crawl topic 우회 / buffer fail fallback / batch mixed / nil buffer Noop / Underlying / Encode-Decode round-triptest/internal/scheduler/buffer_drainer_test.go(5): drain available / skip on threshold / re-enqueue on publish fail / idle empty / nil deps validationBackward compatibility
PUBLISHER_REDIS_BUFFER_ENABLED=false(default) → BufferingProducer wiring 자체 skip, rawKafkaProducer사용 — 기존 동작 100% 보존scheduler.BacklogThrottler는 그대로 유지 — drainer 가 죽거나 buffer 비정상 비활성화 시 fail-safeIngestionLock 와의 관계
기존
internal/locks/ingestion_lock.go의 24h IngestionLock 은 publisher 가 SETNX 로 잡음. buffer 진입 시점도 동일 → buffer 가 dedup 의미 보존, 단지 staging 영역만 추가.Graceful shutdown 동작
sched.Stop()후bufferDrainer.Stop()호출drainAll(부팅 직후 1회) 가 자연 회복 (이슈 [FEATURE] normal/low crawl 토픽 Redis 버퍼링 + 주기적 Kafka backlog drain #510 본문 옵션 A 채택)CI / 머지 게이트 점검
변경 영향 범위
pkg/redis/(Client 메소드 추가)pkg/queue/(BufferingProducer 데코레이터 + JobBuffer interface)internal/scheduler/(BufferDrainer goroutine)pkg/config/processor/(JobBufferConfig + 6 env)pkg/config/internal/parse/(PositiveInt64 helper)cmd/issuetracker/main.go(wiring + Redis 초기화 위치 이동)Medium— opt-in 기본값이라 비활성 시 회귀 위험 0. 활성화 시 publish 경로에 Redis 매개됨 → Redis 장애 = fallback 으로 직접 publish (정상 동작 유지). main.go 의 Redis 초기화 위치 이동이 가장 큰 변경이나 동일 변수 (redisClientShared) 와 동일 fallback 로직 사용.Required Status Checks
Commit LintPR Title LintLinked Issue CheckFormat CheckBuildTestLint롤백 계획
PUBLISHER_REDIS_BUFFER_ENABLED=false설정만으로 즉시 비활성화 — PR revert 불필요🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Chores
.env.examplefor buffer tuning.