Skip to content

Solution (#4143): [BUG] Tracking table items not cleaned after client disconnect #4163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TFGSUMIT wants to merge 1 commit into
base: unstable
Choose a base branch
from
Open

Solution (#4143): [BUG] Tracking table items not cleaned after client disconnect #4163

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#ifndef TRACKING_H
#define TRACKING_H

#include "deps/jemalloc/include/jemalloc/internal/arena_structs.h"

@valkey-review-bot valkey-review-bot Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This pulls in a jemalloc internal header. jemalloc's struct arena_s is a memory-allocator arena and has no relation to the client tracking table. It is also not includable from server code: the path is relative to the repo root (server sources build from src/ without a repo-root include path), the header chains into a dozen other jemalloc/internal/* headers that require jemalloc's configure-generated definitions (jemalloc_internal_defs.h does not exist until the dep is built), and Valkey supports MALLOC=libc builds where jemalloc isn't configured at all. Any state the tracking cleanup needs belongs in src/tracking.c's own structures, not in deps/.


struct tracking_table {

@valkey-review-bot valkey-review-bot Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This header is dead code: no file includes it, src/Makefile and src/CMakeLists.txt never reference it, and tracking_table_defragment/tracking_table_sweep_dead_clients/tracking_table_periodic_sweep are declared here but defined and called nowhere in the tree. struct tracking_table is likewise unused — the server's tracking table is rax *TrackingTable in src/tracking.c:44, which this PR never touches. The file is also misnamed: it landed as a file literally called c at the repository root instead of a header under src/. As-is the change is a no-op with respect to the reported bug; the sweeper needs an implementation in src/tracking.c wired into the disconnect/cron path.

struct arena *arena;
};

void tracking_table_defragment(struct tracking_table *table);
void tracking_table_sweep_dead_clients(struct tracking_table *table);
void tracking_table_periodic_sweep(struct tracking_table *table);
Comment on lines +4 to +12

@coderabbitai coderabbitai Bot Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n '\b(struct arena|struct arena_s|arena_t|tracking_table)\b' .

Repository: valkey-io/valkey

Length of output: 25920

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== file c =="
cat -n c

echo
echo "== jemalloc arena typedefs =="
sed -n '1,80p' deps/jemalloc/include/jemalloc/internal/arena_types.h

echo
echo "== jemalloc arena structs =="
sed -n '1,80p' deps/jemalloc/include/jemalloc/internal/arena_structs.h

Repository: valkey-io/valkey

Length of output: 4922

Use struct arena_s * for the tracking-table arena pointer.

struct arena * is a different incomplete type from jemalloc’s struct arena_s/arena_t, so this won’t match real arena pointers. Forward-declare struct arena_s here and store struct arena_s *arena; instead of including the internal arena definition.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@c` around lines 4 - 12, Update struct tracking_table to use the correct
jemalloc arena type: forward-declare struct arena_s, replace its arena member
with struct arena_s *arena, and remove the unnecessary arena_structs.h include.

Source: MCP tools


#endif /* TRACKING_H */
Loading