Retry mongo writes

[adiom-mark]

Summary by CodeRabbit

• Bug Fixes
  • Added automatic exponential backoff retries (maximum 3 attempts) to write operations for improved resilience.
  • Enhanced error classification to distinguish network unavailability issues from internal errors for better diagnostic accuracy.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Exponential backoff retry logic (max 3 retries) is added around three write paths in the common connector base, with non-retryable errors marked as permanent. Simultaneously, MongoDB connector error handling is refactored to centralize network error detection and mapping via a new maybeUnavailableError function, replacing direct CodeInternal returns.

Changes

Cohort / File(s)	Summary
Retry Logic with Exponential Backoff connectors/common/base.go	Wraps three write paths (WriteUpdates via maybeOptimizedImpl, WriteData via maybeOptimizedImpl, WriteUpdates via impl) with exponential backoff retry mechanism (max 3 retries). Non-retryable errors treated as permanent; retryable errors continue retry loop. Progress updates (UpdateWriteLSN) performed after successful attempts.
Centralized Network Error Mapping connectors/mongo/conn.go	Introduces maybeUnavailableError() function to centralize error classification: maps mongo.IsNetworkError and net.Error cases to CodeUnavailable; defaults to CodeInternal. Adds net import and applies mapping in WriteData and WriteUpdates error paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• connectors/common/base.go: Verify exponential backoff retry implementation across three distinct write paths; confirm non-retryable vs. retryable error distinction and UpdateWriteLSN placement post-success.
• connectors/mongo/conn.go: Validate error classification logic, network error detection accuracy, and proper fallback to CodeInternal.
• Cross-file concern: Ensure retry behavior in base.go doesn't conflict with error mapping in mongo/conn.go during network failures.

Poem

🐰 Three retries and a hop,
Networks stumble, we don't stop,
Backoff whispers, soft and sweet,
Errors sorted, flow complete! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Retry mongo writes' directly and concisely summarizes the main change: introducing exponential backoff retries for MongoDB write operations (WriteData and WriteUpdates).

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch retrymongowrites

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 787132e and fef8d07.

📒 Files selected for processing (2)

• connectors/common/base.go (3 hunks)
• connectors/mongo/conn.go (5 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

connectors/common/base.go (2)

gen/adiom/v1/messages.pb.go (6)

• WriteUpdatesRequest (977-984)
• WriteUpdatesRequest (997-997)
• WriteUpdatesRequest (1012-1014)
• WriteDataRequest (881-888)
• WriteDataRequest (901-901)
• WriteDataRequest (916-918)

protocol/iface/connector.go (1)

• Namespace (104-107)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: build

🔇 Additional comments (8)

connectors/common/base.go (3)

1088-1102: LGTM! Retry logic correctly implemented.

The exponential backoff retry with permanent error handling for non-retryable errors is correctly implemented. Progress tracking happens after successful writes, preserving the existing flow.

---

1110-1124: LGTM! Consistent retry implementation.

The retry logic for WriteData follows the same correct pattern as the WriteUpdates path, with proper handling of non-retryable errors and progress updates after success.

---

1153-1167: LGTM! Complete retry coverage for all write paths.

The final flush path now has the same retry logic as the other write paths, ensuring consistent error handling across all write operations.

connectors/mongo/conn.go (5)

10-10: LGTM! Import needed for network error detection.

The net import is correctly added to support the net.Error interface check in the new maybeUnavailableError function.

---

894-894: LGTM! Network error classification for retries.

The switch to maybeUnavailableError enables proper retry of transient network failures in the write path, working in conjunction with the retry logic added in base.go.

---

905-905: LGTM! Consistent error handling.

Consistent with the previous error path, ensuring all write failures are properly classified for retry logic.

---

976-976: LGTM! Uniform error classification across write operations.

WriteUpdates now uses the same error classification as WriteData, ensuring consistent retry behavior across all write paths.

---

1039-1048: mongo.IsNetworkError API verified.

The function correctly uses mongo.IsNetworkError from the MongoDB Go driver v1.17.4. The error classification logic is sound: network errors are mapped to CodeUnavailable (retryable), while other errors default to CodeInternal. The additional check for net.Error provides defensive coverage.

---

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 and usage tips.}

[coderabbitai]

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)

connectors/common/base.go (1)

1117-1129: Missing retryability check before returning errors.

Same issue as the previous retry blocks - errors are returned without checking isRetryable(err) first. This pattern appears in all three retry blocks added by this PR and should be fixed consistently.

Apply this diff to add the retryability check:

 		backoffConfig := backoff.WithMaxRetries(backoff.NewExponentialBackOff(), 3)
 		if err := backoff.Retry(func() error {
 			_, err := c.impl.WriteUpdates(c.flowCtx, connect.NewRequest(&adiomv1.WriteUpdatesRequest{
 				Namespace: ns,
 				Updates:   msgs,
 				Type:      c.settings.DestinationDataType,
 			}))
+			if err != nil && !isRetryable(err) {
+				return backoff.Permanent(err)
+			}
 			return err
 		}, backoffConfig); err != nil {
 			return err
 		}

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c70e70 and a0fa27e.

📒 Files selected for processing (2)

• connectors/common/base.go (2 hunks)
• connectors/mongo/conn.go (5 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

connectors/common/base.go (2)

gen/adiom/v1/messages.pb.go (6)

• WriteUpdatesRequest (977-984)
• WriteUpdatesRequest (997-997)
• WriteUpdatesRequest (1012-1014)
• WriteDataRequest (881-888)
• WriteDataRequest (901-901)
• WriteDataRequest (916-918)

protocol/iface/connector.go (1)

• Namespace (104-107)

🔇 Additional comments (1)

connectors/mongo/conn.go (1)

1015-1024: LGTM! Error classification correctly maps transient failures.

The maybeUnavailableError function properly centralizes error handling by mapping network-related errors to CodeUnavailable (which is retryable per isRetryable in base.go), while falling back to CodeInternal for other failures. The two-level check (MongoDB-specific network errors, then general net.Error) provides comprehensive coverage of transient network issues.