fix(p2p): resolve map race, nil iterator panic, and retry drops in replicator

[pranav718]

Relevant issue(s)

Resolves #5199

Description

This PR hardens the P2P replicator subsystem (internal/db/p2p/replicator.go) against concurrency races and runtime panics:

1. Fix concurrent map read/write race in pushLogToReplicators: Iterating p.replicators[colID] without holding p.repMu raced with updateReplicators on peer updates. Peer IDs are now safely copied into a local slice under p.repMu.Lock() prior to iteration.
2. Fix nil iterator dereference panic in retryReplicators: Added the missing return statement when p.db.Multistore().Peerstore().Iterator() returns an error, preventing iter.Next() from panicking on a nil pointer.
3. Fix retry drop on network timeout in pushLogToReplicators: When a replication request times out (context.DeadlineExceeded), handleReplicatorFailure is now passed p.ctx instead of the expired network request context, ensuring retry records are properly persisted to the peerstore.
4. Protect handleCompletedReplicatorRetry with handleRetryMutex: Synchronizes peerstore retry record modifications with concurrent failure handlers.
5. Clean up loop context cancellations: Replaced defer cancel() inside head iteration loops with explicit per-iteration cancellations.

Tasks

• Copy peer keys under repMu lock in pushLogToReplicators
• Add missing return on iterator error in retryReplicators
• Decouple failure recovery from expired network context
• Protect handleCompletedReplicatorRetry with handleRetryMutex
• Add regression and race tests in internal/db/p2p/replicator_test.go
• Verify all unit and integration tests pass cleanly with go test -race

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: b5110361-164a-4c50-8bff-92092d1dde6a

📥 Commits

Reviewing files that changed from the base of the PR and between d08080e and fca67e6.

📒 Files selected for processing (2)

• internal/db/p2p/replicator.go
• internal/db/p2p/replicator_test.go

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

📜 Recent review details

🔇 Additional comments (2)

internal/db/p2p/replicator.go (1)

263-277: LGTM!

Also applies to: 388-430, 492-494, 619-619, 891-901

internal/db/p2p/replicator_test.go (1)

43-97: LGTM!

---

📝 Walkthrough

Walkthrough

The P2P replicator now snapshots peer IDs before asynchronous dispatch, cancels request timeout contexts after each request, serializes completed retry handling, and returns on iterator creation errors. New tests cover concurrent map access and canceled retry completion.

Changes

Replicator safety fixes

Layer / File(s)	Summary
Safe log push dispatch internal/db/p2p/replicator.go	pushLogToReplicators copies peer IDs while holding repMu. pushHeadsForDoc explicitly cancels each request timeout context.
Retry state and iterator handling internal/db/p2p/replicator.go	handleCompletedReplicatorRetry serializes retry completion. retryReplicators returns after iterator creation errors. retryDoc explicitly cancels each request timeout context.
Concurrency and cancellation tests internal/db/p2p/replicator_test.go	Test doubles support the P2P tests. Tests exercise concurrent replicator updates and canceled retry completion.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to fca67

This change hardens P2P replication by preventing a map race and iterator panic, preserving retries after timeouts, and synchronizing retry updates. No actionable merge-blocking risk remains beyond normal checks and review.

Assessment against linked issues

Objective	Addressed	Explanation
Concurrent map access, nil iterator handling, and retry completion synchronization [#5199]	✅
Preserve retries after network request timeouts [#5199]	❓	The provided changes show per-request context cancellation in pushHeadsForDoc and retryDoc, but do not show handleReplicatorFailure receiving the long-lived p.ctx required by the issue.

---

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.}