Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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)
✨ 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. Review rate limit: 0/1 reviews remaining, refill in 12 minutes and 58 seconds.Comment |
- A: Remove dead pendingState field from AnycastManager
- B: Fix timer callback mutex re-entry deadlock via channel-based debounce
Timer now signals a debounceCh read by Start() loop instead of
calling announceLocked directly (which would re-enter the non-recursive mutex)
- C: Add trailing newline to gobgp.go
- D: Add ORIGIN and NEXT_HOP path attributes to Withdraw (RFC 4271)
- E: Track announcedVIPs map; Stop() now withdraws all active routes
- F: Implement monitorPeer reconnection — periodic peer health check
via re-add attempt; on failure, restarts BGP server and re-adds peer
Fixes: #112 (Stop withdraw), #113 (Withdraw attributes), #111 (peer reconnection)
- GoBGPAdapter: track Serve() goroutine in WaitGroup for graceful shutdown - GoBGPAdapter: add mutex for concurrent access to config fields - GoBGPAdapter: add monitorPeer goroutine for reconnection mechanism - GoBGPAdapter: Stop() now waits for all goroutines and withdraws active routes - GoBGPAdapter: Withdraw now sends ORIGIN + NEXT_HOP path attributes (RFC 4271) - AnycastManager: add mutex to protect check-then-act on VIP binding - AnycastManager: add channel-based debounce to prevent VIP flapping on health transitions - AnycastManager: rename announce/withdraw to announceLocked/withdrawLocked (internal) - Add ANYCAST_DEBOUNCE env var support (defaults to 5s) - Update NewAnycastManager signature: add debounceDuration parameter - Fix goroutine leak in monitorPeer reconnection error path - Add trailing newlines to gobgp.go and anycast_manager_test.go Fixes: #93, #110, #116, #112, #113, #111
poyrazK
force-pushed
the
fix/bgp-anycast-stability
branch
from
aa91b2b to
70dd570
Compare
- Fix stale "exponential backoff" comment in monitorPeer (no backoff implemented) - Add TestGoBGPAdapter_StopWithdrawsActiveRoutes to verify Stop() clears the announcedVIPs map
- gobgp.go: use parent context (ctx) for WithTimeout instead of context.Background() (contextcheck) - gobgp.go: check error return value of DeletePath in Stop() (errcheck) - anycast_manager.go: remove empty block from timer stop (revive)
1 participant
Summary
Implements 6 BGP/Anycast stability improvements for production DNS anycast deployments:
sync.WaitGroupANYCAST_DEBOUNCE, default 5s)sync.Mutexprotects state transitionsStop()did not withdraw active routes before closing sessionChanges
internal/adapters/routing/gobgp.goServe()goroutine now tracked inWaitGroupfor gracefulStop()SetConfig()protected by mutex for concurrent accessmonitorPeer()goroutine for reconnection mechanismStop()waits for all goroutines viawg.Wait()before closing BGP serverlocalASN,peerASN,peerIPfields for reconnection useinternal/core/services/anycast_manager.gosync.Mutexto protect check-then-act on VIP bindingannounce/withdrawtoannounceLocked/withdrawLocked(caller must hold mutex)TriggerCheck()now locks mutex and uses debounce logicStart()withdraws on shutdowncmd/clouddns/main.goNewAnycastManagercall updated to pass debounce duration (5s default)Test files updated
anycast_manager_test.go: updated constructor calls to 7 args,announce→announceLocked,withdraw→withdrawLockedanycast_test.go: updated constructor call to 7 argsTest plan
go build ./...go test ./...(all pass)go test -race -run "Anycast" ./internal/core/services/... ./internal/dns/server/...(race detector clean)BindVIPcalls do not duplicate, rapid health transitions are debounced, shutdown withdraws routes