[PR #375] [MERGED] Fix connected_count metric underflow in TransportPool::get() #396

New issue

Closed

opened 2026-10-26 09:49:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 09:49:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/deepseek-ai/3FS/pull/375
Author: @yurekami
Created: 1/1/2026
Status: ✅ Merged
Merged: 1/1/2026
Merged by: @SF-Zhou

Base: main ← Head: fix/connected-count-underflow

📝 Commits (1)

450eede Fix connected_count metric underflow in TransportPool::get()

📊 Changes

1 file changed (+5 additions, -1 deletions)

View changed files

📝 src/common/net/TransportPool.cc (+5 -1)

📄 Description

Summary

This PR fixes the common.net.connected_count metric underflow bug reported in #366.

Root Cause

In TransportPool::get(), when creating a new transport, the code always incremented acceptedCount regardless of the addr.ip value:

// Before (bug)
transport = Transport::create(addr, io_worker);
set.add(transport, idx);
acceptedCountRecorder.set(++acceptedCount);  // Always increments acceptedCount

However, TransportPool::add() and TransportPool::remove() correctly check addr.ip:

addr.ip == 0: Use acceptedCount (incoming/accepted connections)
addr.ip != 0: Use connectedCount (outgoing/connected connections)

The Bug

get() creates a transport with addr.ip != 0 → increments acceptedCount (wrong!)
Later, remove() is called for that transport → decrements connectedCount (correct)
connectedCount underflows → wraps to UINT32_MAX (4294967295)

Fix

Added the same addr.ip check that exists in add() and remove():

// After (fix)
transport = Transport::create(addr, io_worker);
set.add(transport, idx);
if (addr.ip == 0) {
  acceptedCountRecorder.set(++acceptedCount);
} else {
  connectedCountRecorder.set(++connectedCount);
}

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/deepseek-ai/3FS/pull/375 **Author:** [@yurekami](https://github.com/yurekami) **Created:** 1/1/2026 **Status:** ✅ Merged **Merged:** 1/1/2026 **Merged by:** [@SF-Zhou](https://github.com/SF-Zhou) **Base:** `main` ← **Head:** `fix/connected-count-underflow` --- ### 📝 Commits (1) - [`450eede`](https://github.com/deepseek-ai/3FS/commit/450eedeef97d91e8db48e595c73fa2ddaa90d604) Fix connected_count metric underflow in TransportPool::get() ### 📊 Changes **1 file changed** (+5 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/common/net/TransportPool.cc` (+5 -1) </details> ### 📄 Description ## Summary This PR fixes the `common.net.connected_count` metric underflow bug reported in #366. ## Root Cause In `TransportPool::get()`, when creating a new transport, the code always incremented `acceptedCount` regardless of the `addr.ip` value: ```cpp // Before (bug) transport = Transport::create(addr, io_worker); set.add(transport, idx); acceptedCountRecorder.set(++acceptedCount); // Always increments acceptedCount ``` However, `TransportPool::add()` and `TransportPool::remove()` correctly check `addr.ip`: - `addr.ip == 0`: Use `acceptedCount` (incoming/accepted connections) - `addr.ip != 0`: Use `connectedCount` (outgoing/connected connections) ## The Bug 1. `get()` creates a transport with `addr.ip != 0` → increments `acceptedCount` (wrong!) 2. Later, `remove()` is called for that transport → decrements `connectedCount` (correct) 3. `connectedCount` underflows → wraps to `UINT32_MAX` (4294967295) ## Fix Added the same `addr.ip` check that exists in `add()` and `remove()`: ```cpp // After (fix) transport = Transport::create(addr, io_worker); set.add(transport, idx); if (addr.ip == 0) { acceptedCountRecorder.set(++acceptedCount); } else { connectedCountRecorder.set(++connectedCount); } ``` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 09:49:52 +00:00