Skip to content

feat: include peer_unresponsive timestamp in GossipStateSummary - #353

Open
mattyg wants to merge 1 commit into
mainfrom
feat/peer_unresponsive-in-network-info
Open

feat: include peer_unresponsive timestamp in GossipStateSummary#353
mattyg wants to merge 1 commit into
mainfrom
feat/peer_unresponsive-in-network-info

Conversation

@mattyg

@mattyg mattyg commented Oct 31, 2025

Copy link
Copy Markdown
Member

Adds peer_unresponsive to GossipStateSummary, which is exposed in holochain's dump_network_metrics call. This is helpful in diagnosing behavior with unresponsive peers.

Putting this field in GossipStateSummary isn't really the right place as it's not specifically related to gossip behavior. I'm not sure if that's worth addressing here.

@cocogitto-bot

cocogitto-bot Bot commented Oct 31, 2025

Copy link
Copy Markdown

✔️ b73c871 - Conventional commits check succeeded.

@coderabbitai

coderabbitai Bot commented Oct 31, 2025

Copy link
Copy Markdown

Walkthrough

Added a new peer_unresponsive field to the PeerMeta struct in the API crate to track peer unresponsiveness status. Updated the gossip summary generation to populate this field by retrieving unresponsive state from the peer metadata store.

Changes

Cohort / File(s) Summary
PeerMeta struct extension
crates/api/src/gossip.rs
Added public field peer_unresponsive: Option<Timestamp> to PeerMeta struct to store unresponsiveness timestamp metadata
Gossip summary population
crates/gossip/src/summary.rs
Updated PeerMeta construction in K2Gossip::summary to load and populate peer_unresponsive field via async call to get_unresponsive(url) from peer_meta_store

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

  • Straightforward field addition across two related files with consistent pattern
  • New async operation introduced (await get_unresponsive()) requires verification of error handling and propagation
  • Consider reviewing the async call semantics and whether error propagation is appropriate in the summary generation context

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
Title Check ✅ Passed The pull request title "feat: include peer_unresponsive timestamp in GossipStateSummary" is directly aligned with the changes made. The PR adds a new peer_unresponsive: Option<Timestamp> field to the PeerMeta struct in the gossip API and populates it within the K2Gossip::summary function. The title is concise, clear, and accurately reflects the primary change without being misleading or overly vague. It uses a conventional feature commit convention and specifically names the key technical detail (peer_unresponsive timestamp) that developers would recognize when reviewing the repository history.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ 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 feat/peer_unresponsive-in-network-info

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

The following will be added to the changelog


[0.3.0-dev.5] - 2025-10-31

Features

  • Include peer_unresponsive timestamp in GossipStateSummary

@cloudflare-workers-and-pages

Copy link
Copy Markdown

Deploying kitsune2 with  Cloudflare Pages  Cloudflare Pages

Latest commit: b73c871
Status: ✅  Deploy successful!
Preview URL: https://a7ff3264.kitsune2.pages.dev
Branch Preview URL: https://feat-peer-unresponsive-in-ne.kitsune2.pages.dev

View logs

@mattyg
mattyg requested review from a team and removed request for a team October 31, 2025 19:59
@mattyg

mattyg commented Nov 3, 2025

Copy link
Copy Markdown
Member Author

I'm thinking it makes sense to pull out the summary of peer meta store now, rather than kick it down the road. I will revise this PR to do so.

@ThetaSinner

Copy link
Copy Markdown
Member

@mattyg Are you wanting a review on this one?

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