Skip to content

rpc pool: drop hive-api.3speak.tv, which never completes a TCP connect - #84

Merged
feruzm merged 1 commit into
mainfrom
chore/drop-unreachable-node
Aug 24, 2026
Merged

rpc pool: drop hive-api.3speak.tv, which never completes a TCP connect#84
feruzm merged 1 commit into
mainfrom
chore/drop-unreachable-node

Conversation

@feruzm

@feruzm feruzm commented Aug 24, 2026

Copy link
Copy Markdown
Member

Closes #83.

hive-api.3speak.tv is in HiveClients.DefaultNodes but does not complete a TCP connect. DNS resolves, the connect never finishes, on 443 or on 80, so every attempt against it costs the full per-node timeout and returns nothing.

Re-checked before making the change, as the issue asks. From two hosts in different regions, three attempts each:

try1 code=000 connect=0.000000s total=10.002781s
try2 code=000 connect=0.000000s total=10.002220s
try3 code=000 connect=0.000000s total=10.002887s

connect never leaves 0 and every request ends at the -m ceiling.

What changed

  • Dropped the entry from HiveClients.DefaultNodes, folding the reason into the hive-api.arcange.eu note above the list, which was added for exactly this failure mode. No replacement node: the remaining public nodes we consider good are already in the pool, while the two others that are missing are missing for a correctness reason the same comment records.
  • DefaultPool_DoesNotCarryTheUnreachableNode now asserts this host's absence next to arcange, plus that the pool has no duplicate entries. Mutation-checked: re-adding the node turns it red, so does duplicating any entry.

Why it is worth a change rather than leaving it to the health tracker

The tracker does park a node after three consecutive failures, but parking is a recovery mechanism, not an exclusion. The park lapses, the node is probed again, then while unparked it sits in config order, which is exactly where an unproven latency profile starts. A node that answers nothing never produces a latency sample to rank on, so it keeps returning to the front of that list. Since NodeHealthTracker gained per-class latency profiles, there are now two profiles per node that can be unproven at once, so the cost of carrying a node that cannot answer is paid twice.

hive-api.3speak.tv resolves but never finishes a connect, on 443 or 80, so
every attempt costs the full per-node timeout and answers nothing. Parking
does not exclude it: the park lapses, the node is probed again, then a node
that answers nothing never produces a latency sample, so it keeps returning
to the front of config order where any unproven profile starts.

The pool guard now covers it alongside arcange and rejects duplicates.
@coderabbitai

coderabbitai Bot commented Aug 24, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 12 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 73250345-df60-4e7b-997a-cbc480e81d65

📥 Commits

Reviewing files that changed from the base of the PR and between fa1db8e and f1d803f.

📒 Files selected for processing (2)
  • dotnet/EcencyApi.Tests/HiveRpcFailoverTests.cs
  • dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs

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.

@qodo-code-review

Copy link
Copy Markdown

ⓘ Your Qodo trial ends soon. Ask your workspace admin to set up billing to keep reviews running after the trial. Manage billing

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 24, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Drop unreachable hive-api.3speak.tv from default RPC node pool

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove hive-api.3speak.tv from HiveClients.DefaultNodes due to non-completing TCP connects.
• Expand default-pool guard test to assert host exclusions and prevent duplicate entries.
• Document why “no response” nodes evade latency ranking and keep resurfacing after parking.
Diagram

graph TD
  App["Ecency API"] --> Client(["HiveRpcClient"]) --> Rpc{{"Public RPC nodes"}}
  Config[/"HiveClients.DefaultNodes"/] --> Client
  Client --> Tracker(["NodeHealthTracker"])
  Tests["HiveRpcFailoverTests"] --> Config

  subgraph Legend
    direction LR
    _svc(["Service"]) ~~~ _cfg[/"Config list"/] ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Permanent denylist in health tracker
  • ➕ Keeps bad nodes out without editing the default pool
  • ➕ Can support environment-specific exclusions
  • ➖ Adds policy/complexity to runtime logic
  • ➖ Requires careful handling to avoid hiding transient outages
2. Preflight connect check on startup (async)
  • ➕ Automatically prunes nodes that can’t establish TCP/TLS
  • ➕ Avoids repeated per-call timeouts on cold start
  • ➖ Still adds complexity and startup latency
  • ➖ Harder to reason about compared to a static curated pool
3. Separate, shorter connect-timeout vs request-timeout
  • ➕ Limits damage from SYN-hang nodes without removing them
  • ➕ Improves worst-case tail latency
  • ➖ May introduce false negatives on slow networks
  • ➖ Does not address “never answers” nodes reappearing after park lapses

Recommendation: Keep the PR’s approach (remove the endpoint from the curated default pool) as the best tradeoff: it’s deterministic, reduces repeated timeout cost immediately, and aligns with the existing commentary about excluding nodes for known failure modes. The runtime/health-tracker alternatives are viable but add complexity for a problem that is currently a known-bad public endpoint.

Files changed (2) +14 / -4

Bug fix (1) +9 / -4
HiveRpcClient.csRemove 3speak endpoint from DefaultNodes and expand exclusion rationale +9/-4

Remove 3speak endpoint from DefaultNodes and expand exclusion rationale

• Drops 'https://hive-api.3speak.tv' from 'HiveClients.DefaultNodes'. Updates the surrounding comment to group it with 'hive-api.arcange.eu' and explains why non-responding nodes degrade pool ordering despite parking behavior.

dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs

Tests (1) +5 / -0
HiveRpcFailoverTests.csHarden default pool guard: assert 3speak exclusion and no duplicates +5/-0

Harden default pool guard: assert 3speak exclusion and no duplicates

• Extends 'DefaultPool_DoesNotCarryTheUnreachableNode' to assert '3speak' is not present alongside 'arcange'. Adds a uniqueness assertion to prevent duplicate entries in 'DefaultNodes'.

dotnet/EcencyApi.Tests/HiveRpcFailoverTests.cs

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Aug 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Added public infrastructure hostname ✗ Dismissed 📘 Rule violation ⛨ Security
Description
The new comment adds hive-api.3speak.tv, a public infrastructure hostname, to repository source.
This violates the requirement that source code and comments not add hostnames or other
infrastructure details.
Code

dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs[437]

+    // hive-api.arcange.eu and hive-api.3speak.tv are absent for the same reason
Relevance

●●● Strong

Recent repository reviews accept concrete source-level compliance fixes; this explicitly violates
the documented hostname prohibition.

PR-#80

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist prohibits any hostname added to source code or comments. The modified comment
explicitly adds hive-api.3speak.tv alongside the existing hostname.

CLAUDE.md: Do Not Introduce Infrastructure Details or Secrets in This Public Repository: CLAUDE.md: Do Not Introduce Infrastructure Details or Secrets in This Public Repository
dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs[437-437]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The changed comment adds the infrastructure hostname `hive-api.3speak.tv` to a public repository.
## Issue Context
PR Compliance ID 1 prohibits adding hostnames and other infrastructure details to source code or comments. Preserve the rationale for excluding unreachable nodes without naming the specific hosts.
## Fix Focus Areas
- dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs[437-437]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Aug 24, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Added public infrastructure hostname ✗ Dismissed 📘 Rule violation ⛨ Security
Description
The new comment adds hive-api.3speak.tv, a public infrastructure hostname, to repository source.
This violates the requirement that source code and comments not add hostnames or other
infrastructure details.
Code

dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs[437]

+    // hive-api.arcange.eu and hive-api.3speak.tv are absent for the same reason
Relevance

●●● Strong

Recent repository reviews accept concrete source-level compliance fixes; this explicitly violates
the documented hostname prohibition.

PR-#80

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The checklist prohibits any hostname added to source code or comments. The modified comment
explicitly adds hive-api.3speak.tv alongside the existing hostname.

CLAUDE.md: Do Not Introduce Infrastructure Details or Secrets in This Public Repository
dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs[437-437]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The changed comment adds the infrastructure hostname `hive-api.3speak.tv` to a public repository.

## Issue Context
PR Compliance ID 1 prohibits adding hostnames and other infrastructure details to source code or comments. Preserve the rationale for excluding unreachable nodes without naming the specific hosts.

## Fix Focus Areas
- dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs[437-437]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: 🚀 Fast: This is a small, localized pool-configuration removal with focused assertions and no security, API, schema, or concurrency impact.

Grey Divider

Tip of the day
💡 Did you know, you can switch off images and animations for a plain-text comment

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread dotnet/EcencyApi/Infrastructure/HiveRpcClient.cs
@feruzm
feruzm merged commit 3cf322f into main Aug 24, 2026
4 checks passed
@feruzm
feruzm deleted the chore/drop-unreachable-node branch August 24, 2026 12:02
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.

Drop hive-api.3speak.tv from the default node pool: it never completes a TCP connect

1 participant