Skip to content

Conversation

sameh-farouk
Copy link
Member

@sameh-farouk sameh-farouk commented Aug 17, 2025

Summary

Enhances the rmb-sdk-go/peer package with resilience, observability, and handler usage best practices.

What's Changed

Documentation Enhancements

  • Added "Handler expectations and recommended patterns" in README.
  • Clarifies server (Router.Serve) vs. client (RpcClient.Call) patterns.

InnerConnection Improvements

  • Added connectionsTotal and observer fields.
  • Buffered channels (writer, reader, outputCh).
  • Context-aware error handling and backpressure logging.
  • Connection stats tracking and handshake timeout increase.

Observability Layer

  • ConnObserver interface.
  • Implementations: noopObserver (silent), logObserver (with sampled logging).

Connection Statistics (ConnStats)

  • Tracks read/write counts and write latencies.
  • Methods: Summary() and Exit() for periodic and exit reports.

Minor Improvements

  • Buffered peer reader channel (size 1024).
  • Deprecated Federation envelope field.

Why This Matters

  • Better throughput and resilience under load.
  • Improved debugging and diagnostics.
  • Clearer guidance for handler usage.

Impact & Notes

  • Backward-compatible.
  • Observability opt-in by default.
  • Minimal performance overhead.

Related issues:

@sameh-farouk sameh-farouk changed the title RMB-Peer: Handle back pressure RMB-Peer: Enable Observability & Better resilience throughput and resilience under load Sep 1, 2025
@sameh-farouk sameh-farouk changed the title RMB-Peer: Enable Observability & Better resilience throughput and resilience under load RMB-Peer: Enable Observability & Better throughput and resilience under load Sep 1, 2025
@sameh-farouk sameh-farouk mentioned this pull request Sep 18, 2025
@@ -594,7 +594,7 @@ func (d *Peer) makeEnvelope(id string, dest uint32, session *string, cmd *string
}
}

env.Federation = destTwin.Relay
env.Federation = destTwin.Relay // this field is deprecated and no longer used by the relay
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so why it is not removed?

Copy link
Member Author

@sameh-farouk sameh-farouk Sep 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope of PR; comment is enough for now


// Reconnections computes the number of reconnection events from a total connection count.
// It returns max(total-1, 0).
func Reconnections(total int64) int64 {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is not it repeated?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what do you mean?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is used in the InnerConnection loop()'s deferred func

@sameh-farouk
Copy link
Member Author

Superseded by: #1437
This PR’s changes have been merged via the unified feature branch in #1437. Merge commit: c753f8d

Reason: We consolidated multiple related PRs to resolve conflicts, preserve consistent behavior, and land the full feature set together (tags, TTL safety, cooldown failover, lifecycle/observability, URL validation, tests/docs).

Action: Closing this PR as superseded. Thanks for the work and discussion here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants