From 1ea5e25c861c80e8ec49a903ced2dba99bcc3dfa Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Wed, 1 Apr 2026 12:05:11 +0000 Subject: [PATCH 1/4] added external pointer to rax, added vset wrapper and stream mutations of data bytes Signed-off-by: Lior Sventitzky --- src/debug.c | 13 + src/rax.c | 63 +++ src/rax.h | 7 + src/stream.h | 8 + src/t_stream.c | 160 +++++++- src/unit/test_stream_tracking.c | 608 ++++++++++++++++++++++++++++ src/unit/test_vset.cpp | 214 ++++++++++ src/vset.c | 130 ++++-- src/vset.h | 1 + tests/unit/type/stream-tracking.tcl | 248 ++++++++++++ 10 files changed, 1413 insertions(+), 39 deletions(-) create mode 100644 src/unit/test_stream_tracking.c create mode 100644 tests/unit/type/stream-tracking.tcl diff --git a/src/debug.c b/src/debug.c index bbb02dc2d25..f77903be1e3 100644 --- a/src/debug.c +++ b/src/debug.c @@ -33,6 +33,7 @@ #include "crc64.h" #include "bio.h" #include "quicklist.h" +#include "stream.h" #include "fpconv_dtoa.h" #include "cluster.h" #include "threads_mngr.h" @@ -995,6 +996,18 @@ void debugCommand(client *c) { } else if (!strcasecmp(objectGetVal(c->argv[1]), "set-disable-deny-scripts") && c->argc == 3) { server.script_disable_deny_script = atoi(objectGetVal(c->argv[2])); addReply(c, shared.ok); + } else if (!strcasecmp(objectGetVal(c->argv[1]), "stream-verify-tracking") && c->argc == 3) { + robj *o = lookupKeyRead(c->db, c->argv[2]); + if (o == NULL || o->type != OBJ_STREAM) { + addReplyError(c, "No such stream key"); + } else { + char errmsg[256]; + if (streamVerifyTracking(objectGetVal(o), errmsg, sizeof(errmsg))) { + addReply(c, shared.ok); + } else { + addReplyError(c, errmsg); + } + } } else if (!strcasecmp(objectGetVal(c->argv[1]), "config-rewrite-force-all") && c->argc == 2) { if (rewriteConfig(server.configfile, 1) == -1) addReplyErrorFormat(c, "CONFIG-REWRITE-FORCE-ALL failed: %s", strerror(errno)); diff --git a/src/rax.c b/src/rax.c index dc22cc5892d..8b4c37c2943 100644 --- a/src/rax.c +++ b/src/rax.c @@ -193,6 +193,7 @@ rax *raxNew(void) { rax->numnodes = 1; rax->head = raxNewNode(0, 0); rax->alloc_size = rax_ptr_alloc_size(rax) + rax_ptr_alloc_size(rax->head); + rax->external_logical_size = NULL; if (rax->head == NULL) { rax_free(rax); return NULL; @@ -201,6 +202,20 @@ rax *raxNew(void) { } } +/* Set external pointer for logical size aggregation. When non-NULL, every + * rax mutation propagates its logical size delta (using raxNodeCurrentLength) + * to *ptr. The current tree's logical size is added immediately so the + * external counter reflects this tree from the moment of attachment. */ +void raxSetExternalLogicalSize(rax *rax, size_t *ptr) { + rax->external_logical_size = ptr; + if (ptr) *ptr += sizeof(*rax) + raxNodeCurrentLength(rax->head); +} + +/* Propagate a logical size delta to the external counter, if set. */ +static inline void raxExternalDelta(rax *rax, int64_t delta) { + if (rax->external_logical_size) *rax->external_logical_size += delta; +} + /* realloc the node to make room for auxiliary data in order * to store an item in that node. On out of memory NULL is returned. */ raxNode *raxReallocForData(raxNode *n, void *data) { @@ -512,10 +527,12 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* Make space for the value pointer if needed. */ if (!h->iskey || (h->isnull && overwrite)) { size_t oldalloc = rax_ptr_alloc_size(h); + size_t oldlogical = raxNodeCurrentLength(h); h = raxReallocForData(h, data); if (h) { memcpy(parentlink, &h, sizeof(h)); rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(h); + raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(h) - oldlogical)); } } if (h == NULL) { @@ -712,6 +729,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** } splitnode->data[0] = h->data[j]; rax->alloc_size += rax_ptr_alloc_size(splitnode); + raxExternalDelta(rax, raxNodeCurrentLength(splitnode)); if (j == 0) { /* 3a: Replace the old node with the split node. */ @@ -737,6 +755,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** parentlink = cp; /* Set parentlink to splitnode parent. */ rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(trimmed); + raxExternalDelta(rax, raxNodeCurrentLength(trimmed)); } /* 4: Create the postfix node: what remains of the original @@ -752,6 +771,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** memcpy(cp, &next, sizeof(next)); rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(postfix); + raxExternalDelta(rax, raxNodeCurrentLength(postfix)); } else { /* 4b: just use next as postfix node. */ postfix = next; @@ -764,6 +784,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* 6. Continue insertion: this will cause the splitnode to * get a new child (the non common character at the currently * inserted key). */ + raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(h)); rax->alloc_size -= rax_ptr_alloc_size(h); rax_free(h); h = splitnode; @@ -804,6 +825,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** memcpy(cp, &next, sizeof(next)); rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(postfix); + raxExternalDelta(rax, raxNodeCurrentLength(postfix)); /* 3: Trim the compressed node. */ trimmed->size = j; @@ -817,6 +839,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** raxSetData(trimmed, aux); } rax->alloc_size += rax_ptr_alloc_size(trimmed); + raxExternalDelta(rax, raxNodeCurrentLength(trimmed)); /* Fix the trimmed node child pointer to point to * the postfix node. */ @@ -826,6 +849,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* Finish! We don't need to continue with the insertion * algorithm for ALGO 2. The key is already inserted. */ rax->numele++; + raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(h)); rax->alloc_size -= rax_ptr_alloc_size(h); rax_free(h); return 1; /* Key inserted. */ @@ -836,6 +860,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** while (i < len) { raxNode *child; size_t oldalloc = rax_ptr_alloc_size(h); + size_t oldlogical = raxNodeCurrentLength(h); /* If this node is going to have a single child, and there * are other characters, so that that would result in a chain @@ -862,9 +887,11 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** } rax->numnodes++; rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(h) + rax_ptr_alloc_size(child); + raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(h) + raxNodeCurrentLength(child) - oldlogical)); h = child; } size_t oldalloc = rax_ptr_alloc_size(h); + size_t oldlogical = raxNodeCurrentLength(h); raxNode *newh = raxReallocForData(h, data); if (newh == NULL) goto oom; h = newh; @@ -872,6 +899,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** raxSetData(h, data); memcpy(parentlink, &h, sizeof(h)); rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(h); + raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(h) - oldlogical)); return 1; /* Element inserted. */ oom: @@ -1041,6 +1069,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { child = h; debugf("Freeing child %p [%.*s] key:%d\n", (void *)child, (int)child->size, (char *)child->data, child->iskey); + raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(child)); rax->alloc_size -= rax_ptr_alloc_size(child); rax_free(child); rax->numnodes--; @@ -1052,8 +1081,10 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { if (child) { debugf("Unlinking child %p from parent %p\n", (void *)child, (void *)h); size_t oldalloc = rax_ptr_alloc_size(h); + size_t oldlogical = raxNodeCurrentLength(h); raxNode *new = raxRemoveChild(h, child); rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(new); + raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(new) - oldlogical)); if (new != h) { raxNode *parent = raxStackPeek(&ts); raxNode **parentlink; @@ -1171,6 +1202,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { new->size = comprsize; rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(new); + raxExternalDelta(rax, raxNodeCurrentLength(new)); /* Scan again, this time to populate the new node content and * to fix the new node child pointer. At the same time we free @@ -1183,6 +1215,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { raxNode **cp = raxNodeLastChildPtr(h); raxNode *tofree = h; memcpy(&h, cp, sizeof(h)); + raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(tofree)); rax->alloc_size -= rax_ptr_alloc_size(tofree); rax_free(tofree); rax->numnodes--; @@ -1225,6 +1258,7 @@ void raxRecursiveFree(rax *rax, raxNode *n, void (*free_callback)(void *)) { } debugnode("free depth-first", n); if (free_callback && n->iskey && !n->isnull) free_callback(raxGetData(n)); + raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(n)); rax_free(n); rax->numnodes--; } @@ -1234,6 +1268,35 @@ void raxRecursiveFree(rax *rax, raxNode *n, void (*free_callback)(void *)) { void raxFreeWithCallback(rax *rax, void (*free_callback)(void *)) { raxRecursiveFree(rax, rax->head, free_callback); assert(rax->numnodes == 0); + raxExternalDelta(rax, -(int64_t)sizeof(*rax)); + rax_free(rax); +} + +/* Same as raxRecursiveFree but the callback receives a context pointer. */ +static void raxRecursiveFreeWithContext(rax *rax, raxNode *n, + void (*free_callback)(void *data, void *ctx), void *ctx) { + debugnode("free traversing", n); + int numchildren = n->iscompr ? 1 : n->size; + raxNode **cp = raxNodeLastChildPtr(n); + while (numchildren--) { + raxNode *child; + memcpy(&child, cp, sizeof(child)); + raxRecursiveFreeWithContext(rax, child, free_callback, ctx); + cp--; + } + debugnode("free depth-first", n); + if (free_callback && n->iskey && !n->isnull) free_callback(raxGetData(n), ctx); + raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(n)); + rax_free(n); + rax->numnodes--; +} + +/* Free a whole radix tree, calling the specified callback with context + * in order to free the auxiliary data. */ +void raxFreeWithCallbackAndContext(rax *rax, void (*free_callback)(void *data, void *ctx), void *ctx) { + raxRecursiveFreeWithContext(rax, rax->head, free_callback, ctx); + assert(rax->numnodes == 0); + raxExternalDelta(rax, -(int64_t)sizeof(*rax)); rax_free(rax); } diff --git a/src/rax.h b/src/rax.h index 2d0c940698a..6a8360e6761 100644 --- a/src/rax.h +++ b/src/rax.h @@ -135,6 +135,11 @@ typedef struct rax { uint64_t numele; /* Number of keys in the tree */ uint64_t numnodes; /* Number of rax nodes in the tree */ size_t alloc_size; /* Total allocation size of the tree in bytes */ + size_t *external_logical_size; /* If non-NULL, logical size deltas + * (using raxNodeCurrentLength) are + * propagated here on every mutation. + * Allows multiple rax trees to aggregate + * their overhead into one counter. */ } rax; /* Stack data structure used by raxLowWalk() in order to, optionally, return @@ -205,6 +210,8 @@ int raxEOF(raxIterator *it); void raxShow(rax *rax); uint64_t raxSize(rax *rax); size_t raxAllocSize(rax *rax); +void raxSetExternalLogicalSize(rax *rax, size_t *ptr); +void raxFreeWithCallbackAndContext(rax *rax, void (*free_callback)(void *data, void *ctx), void *ctx); unsigned long raxTouch(raxNode *n); void raxSetDebugMsg(int onoff); diff --git a/src/stream.h b/src/stream.h index 1e1b9d13ddb..0e7516e9799 100644 --- a/src/stream.h +++ b/src/stream.h @@ -21,6 +21,10 @@ typedef struct stream { streamID first_id; /* The first non-tombstone entry, zero if empty. */ streamID max_deleted_entry_id; /* The maximal ID that was deleted. */ uint64_t entries_added; /* All time count of elements added. */ + size_t tracked_data_bytes; /* Listpack bytes + consumer name SDS bytes. */ + size_t tracked_struct_bytes; /* sizeof(streamCG/NACK/Consumer) for all structs. */ + size_t tracked_rax_overhead; /* Auto-aggregated rax logical size across ALL + * sub-rax trees via external_logical_size pointer. */ } stream; /* We define an iterator to iterate stream items in an abstract way, without @@ -156,5 +160,9 @@ void streamGetEdgeID(stream *s, int first, int skip_tombstones, streamID *edge_i long long streamEstimateDistanceFromFirstEverEntry(stream *s, streamID *id); int64_t streamTrimByLength(stream *s, long long maxlen, int approx); int64_t streamTrimByID(stream *s, streamID minid, int approx); +int streamVerifyTracking(stream *s, char *errmsg, size_t errlen); +void streamFreeNACKWithTracking(void *data, void *ctx); +void streamFreeConsumerWithTracking(void *data, void *ctx); +void streamFreeCGWithTracking(void *data, void *ctx); #endif diff --git a/src/t_stream.c b/src/t_stream.c index 32090fe496a..71cd213bb7e 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -81,17 +81,99 @@ stream *streamNew(void) { s->max_deleted_entry_id.seq = 0; s->max_deleted_entry_id.ms = 0; s->entries_added = 0; + s->tracked_data_bytes = 0; + s->tracked_struct_bytes = 0; + s->tracked_rax_overhead = 0; + raxSetExternalLogicalSize(s->rax, &s->tracked_rax_overhead); s->cgroups = NULL; /* Created on demand to save memory when not used. */ return s; } +/* Free callbacks with stream context for memory tracking during cleanup. */ +static void streamFreeLPWithTracking(void *data, void *ctx) { + stream *s = ctx; + s->tracked_data_bytes -= lpBytes(data); + lpFree(data); +} + +void streamFreeNACKWithTracking(void *data, void *ctx) { + stream *s = ctx; + s->tracked_struct_bytes -= sizeof(streamNACK); + zfree(data); +} + +void streamFreeConsumerWithTracking(void *data, void *ctx) { + stream *s = ctx; + streamConsumer *sc = data; + s->tracked_struct_bytes -= sizeof(streamConsumer); + s->tracked_data_bytes -= sdsReqSize(sdslen(sc->name), sdsType(sc->name)); + raxFree(sc->pel); /* external pointer auto-subtracts from tracked_rax_overhead */ + sdsfree(sc->name); + zfree(sc); +} + +void streamFreeCGWithTracking(void *data, void *ctx) { + stream *s = ctx; + streamCG *cg = data; + s->tracked_struct_bytes -= sizeof(streamCG); + raxFreeWithCallbackAndContext(cg->pel, streamFreeNACKWithTracking, s); + raxFreeWithCallbackAndContext(cg->consumers, streamFreeConsumerWithTracking, s); + zfree(cg); +} + /* Free a stream, including the listpacks stored inside the radix tree. */ void freeStream(stream *s) { - raxFreeWithCallback(s->rax, lpFreeVoid); - if (s->cgroups) raxFreeWithCallback(s->cgroups, streamFreeCGVoid); + raxFreeWithCallbackAndContext(s->rax, streamFreeLPWithTracking, s); + if (s->cgroups) raxFreeWithCallbackAndContext(s->cgroups, streamFreeCGWithTracking, s); zfree(s); } +/* Verify that tracked counters match a full O(n) walk. Returns 1 if correct, + * 0 on mismatch with a description written to errmsg. */ +int streamVerifyTracking(stream *s, char *errmsg, size_t errlen) { + size_t walk_data = 0, walk_struct = 0; + + /* Walk all listpacks in the main rax to compute total data bytes. */ + raxIterator ri; + raxStart(&ri, s->rax); + raxSeek(&ri, "^", NULL, 0); + while (raxNext(&ri)) walk_data += lpBytes((unsigned char *)ri.data); + raxStop(&ri); + + if (s->cgroups) { + raxStart(&ri, s->cgroups); + raxSeek(&ri, "^", NULL, 0); + while (raxNext(&ri)) { + streamCG *cg = ri.data; + walk_struct += sizeof(streamCG); + walk_struct += raxSize(cg->pel) * sizeof(streamNACK); + + raxIterator ci; + raxStart(&ci, cg->consumers); + raxSeek(&ci, "^", NULL, 0); + while (raxNext(&ci)) { + streamConsumer *sc = ci.data; + walk_struct += sizeof(streamConsumer); + walk_data += sdsReqSize(sdslen(sc->name), sdsType(sc->name)); + } + raxStop(&ci); + } + raxStop(&ri); + } + + if (s->tracked_data_bytes != walk_data) { + snprintf(errmsg, errlen, "tracked_data_bytes mismatch: tracked=%zu walk=%zu", + s->tracked_data_bytes, walk_data); + return 0; + } + if (s->tracked_struct_bytes != walk_struct) { + snprintf(errmsg, errlen, "tracked_struct_bytes mismatch: tracked=%zu walk=%zu", + s->tracked_struct_bytes, walk_struct); + return 0; + } + return 1; +} + /* Return the length of a stream. */ unsigned long streamLength(const robj *subject) { stream *s = objectGetVal(subject); @@ -187,6 +269,7 @@ robj *streamDup(robj *o) { memcpy(new_lp, lp, lp_bytes); memcpy(rax_key, ri.key, sizeof(rax_key)); raxInsert(new_s->rax, (unsigned char *)&rax_key, sizeof(rax_key), new_lp, NULL); + new_s->tracked_data_bytes += lp_bytes; } new_s->length = s->length; new_s->first_id = s->first_id; @@ -218,6 +301,7 @@ robj *streamDup(robj *o) { new_nack->delivery_time = nack->delivery_time; new_nack->delivery_count = nack->delivery_count; raxInsert(new_cg->pel, ri_cg_pel.key, sizeof(streamID), new_nack, NULL); + new_s->tracked_struct_bytes += sizeof(streamNACK); } raxStop(&ri_cg_pel); @@ -231,10 +315,13 @@ robj *streamDup(robj *o) { new_consumer = zmalloc(sizeof(*new_consumer)); new_consumer->name = sdsdup(consumer->name); new_consumer->pel = raxNew(); + raxSetExternalLogicalSize(new_consumer->pel, &new_s->tracked_rax_overhead); raxInsert(new_cg->consumers, (unsigned char *)new_consumer->name, sdslen(new_consumer->name), new_consumer, NULL); new_consumer->seen_time = consumer->seen_time; new_consumer->active_time = consumer->active_time; + new_s->tracked_struct_bytes += sizeof(streamConsumer); + new_s->tracked_data_bytes += sdsReqSize(sdslen(new_consumer->name), sdsType(new_consumer->name)); /* Consumer PEL */ raxIterator ri_cpel; @@ -544,6 +631,7 @@ int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_ lp = lpShrinkToFit(lp); if (ri.data != lp) raxInsert(s->rax, ri.key, ri.key_len, lp, NULL); lp = NULL; + lp_bytes = 0; /* Reset baseline for new node tracking delta. */ } } @@ -652,6 +740,13 @@ int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_ /* Insert back into the tree in order to update the listpack pointer. */ if (ri.data != lp) raxInsert(s->rax, (unsigned char *)&rax_key, sizeof(rax_key), lp, NULL); + + /* Track listpack data bytes delta. For new nodes lp_bytes was reset to 0 + * at creation time (or was 0 from the start if the rax was empty), so the + * delta equals lpBytes(lp). For appends to existing nodes the delta is the + * size difference. */ + s->tracked_data_bytes += (int64_t)lpBytes(lp) - (int64_t)lp_bytes; + s->length++; s->entries_added++; s->last_id = id; @@ -749,6 +844,7 @@ int64_t streamTrim(stream *s, streamAddTrimArgs *args) { } if (remove_node) { + s->tracked_data_bytes -= lpBytes(lp); lpFree(lp); raxRemove(s->rax, ri.key, ri.key_len, NULL); raxSeek(&ri, ">=", ri.key, ri.key_len); @@ -762,6 +858,7 @@ int64_t streamTrim(stream *s, streamAddTrimArgs *args) { if (approx) break; /* Now we have to trim entries from within 'lp' */ + size_t lp_before_trim = lpBytes(lp); int64_t deleted_from_lp = 0; p = lpNext(lp, p); /* Skip deleted field. */ @@ -848,6 +945,9 @@ int64_t streamTrim(stream *s, streamAddTrimArgs *args) { /* Update the listpack with the new pointer. */ raxInsert(s->rax, ri.key, ri.key_len, lp, NULL); + /* Track in-place listpack size change from flag/counter modifications. */ + s->tracked_data_bytes += (int64_t)lpBytes(lp) - (int64_t)lp_before_trim; + break; /* If we are here, there was enough to delete in the current node, so no need to go to the next node. */ } @@ -1255,6 +1355,7 @@ void streamIteratorGetField(streamIterator *si, * with GetID(). */ void streamIteratorRemoveEntry(streamIterator *si, streamID *current) { unsigned char *lp = si->lp; + size_t lp_before = lpBytes(lp); int64_t aux; /* We do not really delete the entry here. Instead we mark it as @@ -1273,6 +1374,7 @@ void streamIteratorRemoveEntry(streamIterator *si, streamID *current) { if (aux == 1) { /* If this is the last element in the listpack, we can remove the whole * node. */ + si->stream->tracked_data_bytes -= lp_before; lpFree(lp); raxRemove(si->stream->rax, si->ri.key, si->ri.key_len, NULL); } else { @@ -1282,6 +1384,9 @@ void streamIteratorRemoveEntry(streamIterator *si, streamID *current) { aux = lpGetInteger(p); lp = lpReplaceInteger(lp, &p, aux + 1); + /* Track in-place listpack size change from flag/counter modifications. */ + si->stream->tracked_data_bytes += (int64_t)lpBytes(lp) - (int64_t)lp_before; + /* Update the listpack with the new pointer. */ if (si->lp != lp) raxInsert(si->stream->rax, si->ri.key, si->ri.key_len, lp, NULL); } @@ -1773,6 +1878,13 @@ size_t streamReplyWithRange(client *c, serverPanic("NACK half-created. Should not be possible."); } + /* Track fresh NACK creation (group_inserted == 1). When + * group_inserted == 0 we reused an existing NACK so no new + * struct was allocated. */ + if (group_inserted == 1) { + s->tracked_struct_bytes += sizeof(streamNACK); + } + consumer->active_time = commandTimeSnapshot(); /* Propagate as XCLAIM. */ @@ -2363,6 +2475,11 @@ void xreadCommand(client *c) { if (consumer == NULL) { consumer = streamCreateConsumer(groups[i], objectGetVal(consumername), c->argv[streams_arg + i], c->db->id, SCC_DEFAULT); + if (consumer) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + } if (noack) streamPropagateConsumerCreation(c, spi.keyname, spi.groupname, consumer->name); } consumer->seen_time = commandTimeSnapshot(); @@ -2487,15 +2604,21 @@ static void streamFreeConsumerVoid(void *sc) { * the same name already exists NULL is returned, otherwise the pointer to the * consumer group is returned. */ streamCG *streamCreateCG(stream *s, char *name, size_t namelen, streamID *id, long long entries_read) { - if (s->cgroups == NULL) s->cgroups = raxNew(); + if (s->cgroups == NULL) { + s->cgroups = raxNew(); + raxSetExternalLogicalSize(s->cgroups, &s->tracked_rax_overhead); + } if (raxFind(s->cgroups, (unsigned char *)name, namelen, NULL)) return NULL; streamCG *cg = zmalloc(sizeof(*cg)); cg->pel = raxNew(); + raxSetExternalLogicalSize(cg->pel, &s->tracked_rax_overhead); cg->consumers = raxNew(); + raxSetExternalLogicalSize(cg->consumers, &s->tracked_rax_overhead); cg->last_id = *id; cg->entries_read = entries_read; raxInsert(s->cgroups, (unsigned char *)name, namelen, cg, NULL); + s->tracked_struct_bytes += sizeof(streamCG); return cg; } @@ -2529,12 +2652,13 @@ streamConsumer *streamCreateConsumer(streamCG *cg, sds name, robj *key, int dbid int notify = !(flags & SCC_NO_NOTIFY); int dirty = !(flags & SCC_NO_DIRTIFY); streamConsumer *consumer = zmalloc(sizeof(*consumer)); + consumer->name = sdsdup(name); int success = raxTryInsert(cg->consumers, (unsigned char *)name, sdslen(name), consumer, NULL); if (!success) { + sdsfree(consumer->name); zfree(consumer); return NULL; } - consumer->name = sdsdup(name); consumer->pel = raxNew(); consumer->active_time = -1; consumer->seen_time = commandTimeSnapshot(); @@ -2706,7 +2830,10 @@ void xgroupCommand(client *c) { } else if (!strcasecmp(opt, "DESTROY") && c->argc == 4) { if (cg) { raxRemove(s->cgroups, (unsigned char *)grpname, sdslen(grpname), NULL); - streamFreeCG(cg); + s->tracked_struct_bytes -= sizeof(streamCG); + raxFreeWithCallbackAndContext(cg->pel, streamFreeNACKWithTracking, s); + raxFreeWithCallbackAndContext(cg->consumers, streamFreeConsumerWithTracking, s); + zfree(cg); server.dirty++; notifyKeyspaceEvent(NOTIFY_STREAM, "xgroup-destroy", c->argv[2], c->db->id); addReply(c, shared.cone); @@ -2717,6 +2844,11 @@ void xgroupCommand(client *c) { } } else if (!strcasecmp(opt, "CREATECONSUMER") && c->argc == 5) { streamConsumer *created = streamCreateConsumer(cg, objectGetVal(c->argv[4]), c->argv[2], c->db->id, SCC_DEFAULT); + if (created) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(created->name), sdsType(created->name)); + raxSetExternalLogicalSize(created->pel, &s->tracked_rax_overhead); + } addReplyLongLong(c, created ? 1 : 0); } else if (!strcasecmp(opt, "DELCONSUMER") && c->argc == 5) { long long pending = 0; @@ -2725,6 +2857,8 @@ void xgroupCommand(client *c) { /* Delete the consumer and returns the number of pending messages * that were yet associated with such a consumer. */ pending = raxSize(consumer->pel); + s->tracked_struct_bytes -= sizeof(streamConsumer) + pending * sizeof(streamNACK); + s->tracked_data_bytes -= sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); streamDelConsumer(cg, consumer); server.dirty++; notifyKeyspaceEvent(NOTIFY_STREAM, "xgroup-delconsumer", c->argv[2], c->db->id); @@ -2855,6 +2989,7 @@ void xackCommand(client *c) { raxRemove(group->pel, buf, sizeof(buf), NULL); raxRemove(nack->consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); + ((stream *)objectGetVal(o))->tracked_struct_bytes -= sizeof(streamNACK); acknowledged++; server.dirty++; } @@ -3209,9 +3344,15 @@ void xclaimCommand(client *c) { } /* Do the actual claiming. */ + stream *s = objectGetVal(o); streamConsumer *consumer = streamLookupConsumer(group, objectGetVal(c->argv[3])); if (consumer == NULL) { consumer = streamCreateConsumer(group, objectGetVal(c->argv[3]), c->argv[1], c->db->id, SCC_DEFAULT); + if (consumer) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + } } consumer->seen_time = commandTimeSnapshot(); @@ -3239,6 +3380,7 @@ void xclaimCommand(client *c) { raxRemove(group->pel, buf, sizeof(buf), NULL); raxRemove(nack->consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); + s->tracked_struct_bytes -= sizeof(streamNACK); } continue; } @@ -3252,6 +3394,7 @@ void xclaimCommand(client *c) { /* Create the NACK. */ nack = streamCreateNACK(NULL); raxInsert(group->pel, buf, sizeof(buf), nack, NULL); + s->tracked_struct_bytes += sizeof(streamNACK); } if (nack != NULL) { @@ -3388,9 +3531,15 @@ void xautoclaimCommand(client *c) { } /* Do the actual claiming. */ + stream *s = objectGetVal(o); streamConsumer *consumer = streamLookupConsumer(group, objectGetVal(c->argv[3])); if (consumer == NULL) { consumer = streamCreateConsumer(group, objectGetVal(c->argv[3]), c->argv[1], c->db->id, SCC_DEFAULT); + if (consumer) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + } } consumer->seen_time = commandTimeSnapshot(); @@ -3425,6 +3574,7 @@ void xautoclaimCommand(client *c) { raxRemove(group->pel, ri.key, ri.key_len, NULL); raxRemove(nack->consumer->pel, ri.key, ri.key_len, NULL); streamFreeNACK(nack); + s->tracked_struct_bytes -= sizeof(streamNACK); /* Remember the ID for later */ deleted_ids[deleted_id_num++] = id; raxSeek(&ri, ">=", ri.key, ri.key_len); diff --git a/src/unit/test_stream_tracking.c b/src/unit/test_stream_tracking.c new file mode 100644 index 00000000000..c45aac58b75 --- /dev/null +++ b/src/unit/test_stream_tracking.c @@ -0,0 +1,608 @@ +/* + * Copyright (c) Valkey Contributors + * All rights reserved. + * SPDX-License-Identifier: BSD-3-Clause + * + * Unit tests for stream tracked_data_bytes, tracked_struct_bytes, + * and tracked_rax_overhead (Approach F: external pointer). + */ + +#include "../fmacros.h" +#include "../server.h" +#include "../stream.h" +#include "../rax.h" +#include "../listpack.h" +#include "../sds.h" +#include "test_help.h" + +#include +#include +#include +#include +#include + +/* ── Forward declarations for internal functions ─────────────────────── */ +int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_id, streamID *use_id, int seq_given); +streamCG *streamCreateCG(stream *s, char *name, size_t namelen, streamID *id, long long entries_read); +streamConsumer *streamCreateConsumer(streamCG *cg, sds name, robj *key, int dbid, int flags); +streamNACK *streamCreateNACK(streamConsumer *consumer); +void streamFreeNACK(streamNACK *na); +void streamDelConsumer(streamCG *cg, streamConsumer *consumer); +void freeStream(stream *s); +int64_t streamTrimByLength(stream *s, long long maxlen, int approx); +int64_t streamTrimByID(stream *s, streamID minid, int approx); +void streamEncodeID(void *buf, streamID *id); +robj *streamDup(robj *o); + +/* ── Ground truth: walk everything and compute sizes ─────────────────── */ + +static size_t computeDataBytesWalk(stream *s) { + size_t total = 0; + raxIterator ri; + raxStart(&ri, s->rax); + raxSeek(&ri, "^", NULL, 0); + while (raxNext(&ri)) total += lpBytes((unsigned char *)ri.data); + raxStop(&ri); + if (s->cgroups) { + raxStart(&ri, s->cgroups); + raxSeek(&ri, "^", NULL, 0); + while (raxNext(&ri)) { + streamCG *cg = ri.data; + raxIterator ci; + raxStart(&ci, cg->consumers); + raxSeek(&ci, "^", NULL, 0); + while (raxNext(&ci)) { + streamConsumer *sc = ci.data; + total += sdsReqSize(sdslen(sc->name), sdsType(sc->name)); + } + raxStop(&ci); + } + raxStop(&ri); + } + return total; +} + +static size_t computeStructBytesWalk(stream *s) { + size_t total = 0; + if (s->cgroups) { + raxIterator ri; + raxStart(&ri, s->cgroups); + raxSeek(&ri, "^", NULL, 0); + while (raxNext(&ri)) { + streamCG *cg = ri.data; + total += sizeof(streamCG); + total += raxSize(cg->pel) * sizeof(streamNACK); + raxIterator ci; + raxStart(&ci, cg->consumers); + raxSeek(&ci, "^", NULL, 0); + while (raxNext(&ci)) { + total += sizeof(streamConsumer); + } + raxStop(&ci); + } + raxStop(&ri); + } + return total; +} + +#define ASSERT_STREAM_TRACKING(s) \ + do { \ + size_t wd = computeDataBytesWalk(s); \ + size_t ws = computeStructBytesWalk(s); \ + if ((s)->tracked_data_bytes != wd) { \ + printf("tracked_data_bytes mismatch: tracked=%zu walk=%zu\n", \ + (s)->tracked_data_bytes, wd); \ + TEST_ASSERT(0); \ + } \ + if ((s)->tracked_struct_bytes != ws) { \ + printf("tracked_struct_bytes mismatch: tracked=%zu walk=%zu\n", \ + (s)->tracked_struct_bytes, ws); \ + TEST_ASSERT(0); \ + } \ + } while (0) + +/* ── Helpers ─────────────────────────────────────────────────────────── */ + +static streamID appendEntry(stream *s, const char *field, const char *value) { + robj *argv[2]; + argv[0] = createStringObject(field, strlen(field)); + argv[1] = createStringObject(value, strlen(value)); + streamID id; + streamAppendItem(s, argv, 1, &id, NULL, 0); + decrRefCount(argv[0]); + decrRefCount(argv[1]); + return id; +} + +/* ── Tests ───────────────────────────────────────────────────────────── */ + +int test_stream_tracking_append(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + server.stream_node_max_entries = 10; + + for (int i = 0; i < 100; i++) { + char f[16], v[32]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "value_%d_data", i); + appendEntry(s, f, v); + ASSERT_STREAM_TRACKING(s); + } + /* With max_entries=10, 100 entries must span multiple rax nodes */ + TEST_ASSERT(raxSize(s->rax) == 10); + + server.stream_node_max_entries = 100; + freeStream(s); + return 0; +} + +int test_stream_tracking_trim(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + server.stream_node_max_entries = 10; + + for (int i = 0; i < 100; i++) { + char f[16], v[64]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "value_%d_with_padding", i); + appendEntry(s, f, v); + } + ASSERT_STREAM_TRACKING(s); + + streamTrimByLength(s, 5, 0); + ASSERT_STREAM_TRACKING(s); + + server.stream_node_max_entries = 100; + freeStream(s); + return 0; +} + +int test_stream_tracking_trim_by_id(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + server.stream_node_max_entries = 10; + + streamID ids[50]; + for (int i = 0; i < 50; i++) { + char f[16], v[32]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "value_%d", i); + ids[i] = appendEntry(s, f, v); + } + ASSERT_STREAM_TRACKING(s); + TEST_ASSERT(raxSize(s->rax) == 5); + + streamTrimByID(s, ids[35], 0); + ASSERT_STREAM_TRACKING(s); + TEST_ASSERT(raxSize(s->rax) < 5); + + server.stream_node_max_entries = 100; + freeStream(s); + return 0; +} + +/* Listpack stores integers in variable-width encoding: values 0-127 use + * 7-bit (1 byte), 128+ use 13-bit (2 bytes). The "valid entries" counter + * crosses 128→127 during trim, causing a 1-byte lpBytes change. */ +int test_stream_tracking_trim_encoding_boundary(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + server.stream_node_max_entries = 200; + server.stream_node_max_bytes = 0; + + for (int i = 0; i < 129; i++) { + char f[16], v[16]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "v%d", i); + appendEntry(s, f, v); + } + ASSERT_STREAM_TRACKING(s); + TEST_ASSERT(raxSize(s->rax) == 1); + + size_t bytes_before = s->tracked_data_bytes; + streamTrimByLength(s, 127, 0); + ASSERT_STREAM_TRACKING(s); + /* Counter crossed 128→127, encoding change causes 1-byte decrease */ + TEST_ASSERT(bytes_before - s->tracked_data_bytes == 1); + + server.stream_node_max_entries = 100; + server.stream_node_max_bytes = 4096; + freeStream(s); + return 0; +} + +int test_stream_tracking_iterator_remove(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + + streamID ids[5]; + for (int i = 0; i < 5; i++) { + char f[16], v[16]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "v%d", i); + ids[i] = appendEntry(s, f, v); + } + + for (int i = 0; i < 5; i++) { + streamIterator si; + streamIteratorStart(&si, s, &ids[i], &ids[i], 0); + streamID myid; + int64_t numfields; + if (streamIteratorGetID(&si, &myid, &numfields)) { + streamIteratorRemoveEntry(&si, &myid); + } + streamIteratorStop(&si); + ASSERT_STREAM_TRACKING(s); + } + TEST_ASSERT(s->tracked_data_bytes == 0); + + freeStream(s); + return 0; +} + +int test_stream_tracking_cg_create(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + appendEntry(s, "f", "v"); + + streamID zero = {0, 0}; + streamCreateCG(s, (char *)"grp1", 4, &zero, 0); + streamCreateCG(s, (char *)"grp2", 4, &zero, 0); + ASSERT_STREAM_TRACKING(s); + + freeStream(s); + return 0; +} + +int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + ASSERT_STREAM_TRACKING(s); + + streamID ids[30]; + for (int i = 0; i < 30; i++) { + char f[16], v[32]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "val_%d", i); + ids[i] = appendEntry(s, f, v); + } + ASSERT_STREAM_TRACKING(s); + + streamID zero = {0, 0}; + streamCG *cg = streamCreateCG(s, (char *)"workers", 7, &zero, 0); + ASSERT_STREAM_TRACKING(s); + + /* Create consumers — mirrors command handler tracking. */ + robj *key = createStringObject("mystream", 8); + sds name1 = sdsnew("alice"); + sds name2 = sdsnew("bob_with_longer_name"); + streamConsumer *c1 = streamCreateConsumer(cg, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + if (c1) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); + raxSetExternalLogicalSize(c1->pel, &s->tracked_rax_overhead); + } + streamConsumer *c2 = streamCreateConsumer(cg, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + if (c2) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); + raxSetExternalLogicalSize(c2->pel, &s->tracked_rax_overhead); + } + ASSERT_STREAM_TRACKING(s); + + /* Deliver NACKs — mirrors streamReplyWithRange (XREADGROUP). */ + for (int i = 0; i < 10; i++) { + streamNACK *nack = streamCreateNACK(c1); + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &ids[i]); + raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); + raxInsert(c1->pel, buf, sizeof(buf), nack, NULL); + s->tracked_struct_bytes += sizeof(streamNACK); + } + ASSERT_STREAM_TRACKING(s); + + /* ACK 5 NACKs — mirrors xackCommand. */ + for (int i = 0; i < 5; i++) { + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &ids[i]); + void *result; + raxFind(cg->pel, buf, sizeof(buf), &result); + raxRemove(cg->pel, buf, sizeof(buf), NULL); + raxRemove(c1->pel, buf, sizeof(buf), NULL); + streamFreeNACK((streamNACK *)result); + s->tracked_struct_bytes -= sizeof(streamNACK); + } + ASSERT_STREAM_TRACKING(s); + + /* Delete consumer c2 (0 NACKs) — mirrors DELCONSUMER. */ + s->tracked_struct_bytes -= sizeof(streamConsumer); + s->tracked_data_bytes -= sdsReqSize(sdslen(c2->name), sdsType(c2->name)); + streamDelConsumer(cg, c2); + ASSERT_STREAM_TRACKING(s); + + /* Trim */ + streamTrimByLength(s, 10, 0); + ASSERT_STREAM_TRACKING(s); + + sdsfree(name1); + sdsfree(name2); + decrRefCount(key); + freeStream(s); + return 0; +} + +int test_stream_tracking_destroy_cg(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + appendEntry(s, "f", "v"); + + streamID zero = {0, 0}; + streamCreateCG(s, (char *)"grp1", 4, &zero, 0); + streamCG *cg2 = streamCreateCG(s, (char *)"grp2", 4, &zero, 0); + + /* Create consumers — mirrors command handler tracking. */ + robj *key = createStringObject("mystream", 8); + sds name1 = sdsnew("worker_alpha"); + sds name2 = sdsnew("worker_beta_longer"); + streamConsumer *c1 = streamCreateConsumer(cg2, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); + raxSetExternalLogicalSize(c1->pel, &s->tracked_rax_overhead); + streamConsumer *c2 = streamCreateConsumer(cg2, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); + raxSetExternalLogicalSize(c2->pel, &s->tracked_rax_overhead); + + streamID ids[10]; + for (int i = 0; i < 10; i++) { + char f[16], v[16]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "v%d", i); + ids[i] = appendEntry(s, f, v); + } + /* Deliver NACKs — mirrors streamReplyWithRange. */ + for (int i = 0; i < 7; i++) { + streamNACK *nack = streamCreateNACK(i < 4 ? c1 : c2); + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &ids[i]); + streamConsumer *target = (i < 4 ? c1 : c2); + raxInsert(cg2->pel, buf, sizeof(buf), nack, NULL); + raxInsert(target->pel, buf, sizeof(buf), nack, NULL); + s->tracked_struct_bytes += sizeof(streamNACK); + } + ASSERT_STREAM_TRACKING(s); + + /* Destroy cg2 — mirrors xgroupCommand DESTROY. */ + raxRemove(s->cgroups, (unsigned char *)"grp2", 4, NULL); + s->tracked_struct_bytes -= sizeof(streamCG); + raxFreeWithCallbackAndContext(cg2->pel, streamFreeNACKWithTracking, s); + raxFreeWithCallbackAndContext(cg2->consumers, streamFreeConsumerWithTracking, s); + zfree(cg2); + ASSERT_STREAM_TRACKING(s); + + /* grp1 still exists */ + TEST_ASSERT(s->tracked_struct_bytes > 0); + + sdsfree(name1); + sdsfree(name2); + decrRefCount(key); + freeStream(s); + return 0; +} + +int test_stream_tracking_del_consumer(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + appendEntry(s, "f", "v"); + + streamID zero = {0, 0}; + streamCG *cg = streamCreateCG(s, (char *)"grp", 3, &zero, 0); + + /* Create consumer — mirrors command handler tracking. */ + robj *key = createStringObject("mystream", 8); + sds name = sdsnew("busy_consumer"); + streamConsumer *consumer = streamCreateConsumer(cg, name, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + + /* Deliver 8 NACKs — mirrors streamReplyWithRange. */ + streamID ids[8]; + for (int i = 0; i < 8; i++) { + char f[16], v[16]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "v%d", i); + ids[i] = appendEntry(s, f, v); + + streamNACK *nack = streamCreateNACK(consumer); + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &ids[i]); + raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); + raxInsert(consumer->pel, buf, sizeof(buf), nack, NULL); + s->tracked_struct_bytes += sizeof(streamNACK); + } + ASSERT_STREAM_TRACKING(s); + + /* Delete consumer — mirrors xgroupCommand DELCONSUMER. */ + long long pending = raxSize(consumer->pel); + s->tracked_struct_bytes -= sizeof(streamConsumer) + pending * sizeof(streamNACK); + s->tracked_data_bytes -= sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); + streamDelConsumer(cg, consumer); + ASSERT_STREAM_TRACKING(s); + + sdsfree(name); + decrRefCount(key); + freeStream(s); + return 0; +} + +int test_stream_tracking_dup(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + stream *s = streamNew(); + + for (int i = 0; i < 20; i++) { + char f[16], v[32]; + snprintf(f, sizeof(f), "f%d", i); + snprintf(v, sizeof(v), "value_%d", i); + appendEntry(s, f, v); + } + + streamID zero = {0, 0}; + streamCG *cg = streamCreateCG(s, (char *)"grp", 3, &zero, 0); + + /* Create consumer — mirrors command handler tracking. */ + robj *key = createStringObject("mystream", 8); + sds name = sdsnew("consumer_one"); + streamConsumer *consumer = streamCreateConsumer(cg, name, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + + /* Deliver NACKs — mirrors streamReplyWithRange. */ + streamID ids[5]; + for (int i = 0; i < 5; i++) { + char f[16], v[16]; + snprintf(f, sizeof(f), "nf%d", i); + snprintf(v, sizeof(v), "nv%d", i); + ids[i] = appendEntry(s, f, v); + + streamNACK *nack = streamCreateNACK(consumer); + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &ids[i]); + raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); + raxInsert(consumer->pel, buf, sizeof(buf), nack, NULL); + s->tracked_struct_bytes += sizeof(streamNACK); + } + ASSERT_STREAM_TRACKING(s); + + /* Duplicate */ + robj *orig = createStreamObject(); + freeStream(objectGetVal(orig)); + objectSetVal(orig, s); + + robj *copy = streamDup(orig); + stream *new_s = objectGetVal(copy); + + ASSERT_STREAM_TRACKING(new_s); + TEST_ASSERT(s->tracked_data_bytes == new_s->tracked_data_bytes); + TEST_ASSERT(s->tracked_struct_bytes == new_s->tracked_struct_bytes); + + objectSetVal(orig, NULL); /* prevent double-free of s */ + decrRefCount(orig); + decrRefCount(copy); + sdsfree(name); + decrRefCount(key); + freeStream(s); + return 0; +} + +int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + unsigned seed = (unsigned)time(NULL) ^ (unsigned)getpid(); + srand(seed); + printf(" Fuzzer seed: %u\n", seed); + + stream *s = streamNew(); + robj *key = createStringObject("mystream", 8); + streamID zero = {0, 0}; + + streamCG *cgs[3]; + for (int i = 0; i < 3; i++) { + char cgname[16]; + snprintf(cgname, sizeof(cgname), "cg%d", i); + cgs[i] = streamCreateCG(s, cgname, strlen(cgname), &zero, 0); + } + ASSERT_STREAM_TRACKING(s); + + streamID ids[2048]; + int id_count = 0; + typedef struct { int cg_idx; streamID id; } pending_t; + pending_t pending[4096]; + int pending_count = 0; + + const int NUM_OPS = 2000; + for (int op = 0; op < NUM_OPS; op++) { + int action = rand() % 100; + + if (action < 40 || id_count == 0) { + /* XADD */ + if (id_count < 2048) { + char f[32]; + snprintf(f, sizeof(f), "field_%d", op); + size_t vlen = 5 + ((size_t)rand() % 50); + char val[64]; + memset(val, 'a' + (rand() % 26), vlen); + val[vlen] = '\0'; + ids[id_count] = appendEntry(s, f, val); + id_count++; + } + } else if (action < 55 && id_count > 20) { + /* XTRIM */ + streamTrimByLength(s, id_count / 2, 0); + } else if (action < 70) { + /* Create consumer — mirrors command handler tracking. */ + int cg_idx = rand() % 3; + char cname[32]; + snprintf(cname, sizeof(cname), "consumer_%d_%d", op, rand() % 1000); + sds sname = sdsnew(cname); + streamConsumer *c = streamCreateConsumer(cgs[cg_idx], sname, key, 0, + SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + if (c) { + s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c->name), sdsType(c->name)); + raxSetExternalLogicalSize(c->pel, &s->tracked_rax_overhead); + } + sdsfree(sname); + } else if (action < 85 && id_count > 0) { + /* Deliver NACK — mirrors streamReplyWithRange. */ + int cg_idx = rand() % 3; + if (raxSize(cgs[cg_idx]->consumers) > 0) { + raxIterator ci; + raxStart(&ci, cgs[cg_idx]->consumers); + raxSeek(&ci, "^", NULL, 0); + raxNext(&ci); + streamConsumer *c = ci.data; + raxStop(&ci); + + int idx = rand() % id_count; + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &ids[idx]); + streamNACK *nack = streamCreateNACK(c); + if (raxTryInsert(cgs[cg_idx]->pel, buf, sizeof(buf), nack, NULL)) { + raxInsert(c->pel, buf, sizeof(buf), nack, NULL); + s->tracked_struct_bytes += sizeof(streamNACK); + if (pending_count < 4096) { + pending[pending_count].cg_idx = cg_idx; + pending[pending_count].id = ids[idx]; + pending_count++; + } + } else { + streamFreeNACK(nack); + } + } + } else if (pending_count > 0) { + /* ACK — mirrors xackCommand. */ + int idx = rand() % pending_count; + int cg_idx = pending[idx].cg_idx; + unsigned char buf[sizeof(streamID)]; + streamEncodeID(buf, &pending[idx].id); + void *result; + if (raxFind(cgs[cg_idx]->pel, buf, sizeof(buf), &result)) { + streamNACK *nack = result; + streamConsumer *nack_consumer = nack->consumer; + raxRemove(cgs[cg_idx]->pel, buf, sizeof(buf), NULL); + raxRemove(nack_consumer->pel, buf, sizeof(buf), NULL); + streamFreeNACK(nack); + s->tracked_struct_bytes -= sizeof(streamNACK); + } + pending[idx] = pending[--pending_count]; + } + + if (op % 50 == 0) ASSERT_STREAM_TRACKING(s); + } + ASSERT_STREAM_TRACKING(s); + + decrRefCount(key); + freeStream(s); + return 0; +} diff --git a/src/unit/test_vset.cpp b/src/unit/test_vset.cpp index 8c9c949ff91..ccc0b34dc65 100644 --- a/src/unit/test_vset.cpp +++ b/src/unit/test_vset.cpp @@ -479,3 +479,217 @@ TEST_F(VsetTest, TestVsetFuzzer) { ASSERT_TRUE(vsetIsEmpty(&set) && mock_entry_count == 0); vsetRelease(&set); } + +/* ── Tracking tests ─────────────────────────────────────────────────── */ + +#define ASSERT_VSET_TRACKING(set) \ + do { \ + char errmsg[256]; \ + if (!vsetVerifyTracking(set, errmsg, sizeof(errmsg))) { \ + TEST_PRINT_ERROR(errmsg); \ + return 1; \ + } \ + } while (0) + +/* Force promotion to RAX by inserting enough entries with spread expiries */ +static void fillToRax(vset *set, int count, long long base_expiry) { + for (int i = 0; i < count; i++) { + insert_mock_entry_with_expiry(set, base_expiry + i * 100); + } +} + +int test_vset_tracking_add_to_rax(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + for (int i = 0; i < 100; i++) { + insert_mock_entry_with_expiry(&set, 50000 + i * 100); + ASSERT_VSET_TRACKING(&set); + } + + vsetClear(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_remove(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + while (mock_entry_count > 0) { + remove_mock_entry(&set); + ASSERT_VSET_TRACKING(&set); + } + + vsetRelease(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_expire(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + for (long long now = 2000; now < 30000; now += 2000) { + expire_mock_entries(&set, now); + ASSERT_VSET_TRACKING(&set); + } + + expire_mock_entries(&set, LONG_LONG_MAX); + vsetRelease(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_update(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + for (int i = 0; i < 200; i++) { + update_mock_entry(&set); + ASSERT_VSET_TRACKING(&set); + } + + vsetClear(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_same_bucket_promotion(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + /* All entries within same 8192ms time window */ + for (int i = 0; i < 128; i++) { + insert_mock_entry_with_expiry(&set, 1000 + i); + } + ASSERT_VSET_TRACKING(&set); + + for (int i = 0; i < 50; i++) { + insert_mock_entry_with_expiry(&set, 2000 + i); + ASSERT_VSET_TRACKING(&set); + } + + vsetClear(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_vector_to_hashtable(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + /* Promote to RAX with spread entries */ + for (int i = 0; i < 128; i++) { + insert_mock_entry_with_expiry(&set, 1000 + i); + } + ASSERT_VSET_TRACKING(&set); + + /* Add entries in same fine-grained bucket (16ms window) to force HT */ + for (int i = 0; i < 128; i++) { + insert_mock_entry_with_expiry(&set, 1000); + ASSERT_VSET_TRACKING(&set); + } + + vsetClear(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_defrag(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + TEST_ASSERT(defrag_vset(&set, 0, 0) == 0); + ASSERT_VSET_TRACKING(&set); + + for (int i = 0; i < 50; i++) { + insert_mock_entry_with_expiry(&set, 50000 + i * 100); + } + ASSERT_VSET_TRACKING(&set); + + vsetClear(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_shrink(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + while (mock_entry_count > 0) { + remove_mock_entry(&set); + ASSERT_VSET_TRACKING(&set); + } + + vsetRelease(&set); + free_mock_entries(); + return 0; +} + +int test_vset_tracking_fuzzer(int argc, char **argv, int flags) { + UNUSED(argc); UNUSED(argv); UNUSED(flags); + unsigned seed = (unsigned)time(NULL) ^ (unsigned)getpid(); + srand(seed); + printf(" Vset tracking fuzzer seed: %u\n", seed); + + vset set; + vsetInit(&set); + + fillToRax(&set, 200, 1000); + ASSERT_VSET_TRACKING(&set); + + for (int i = 0; i < 10000; i++) { + int op = rand() % 5; + switch (op) { + case 0: + case 1: + insert_mock_entry(&set); + break; + case 2: + update_mock_entry(&set); + break; + case 3: + remove_mock_entry(&set); + break; + case 4: { + mstime_t now = rand() % 10000; + expire_mock_entries(&set, now); + break; + } + } + if (i % 50 == 0) ASSERT_VSET_TRACKING(&set); + } + ASSERT_VSET_TRACKING(&set); + + expire_mock_entries(&set, LONG_LONG_MAX); + vsetRelease(&set); + free_mock_entries(); + return 0; +} diff --git a/src/vset.c b/src/vset.c index f2a32dcbb46..8bafee11d4b 100644 --- a/src/vset.c +++ b/src/vset.c @@ -802,9 +802,20 @@ static inline hashtable *vsetBucketHashtable(vsetBucket *b) { return (hashtable *)vsetBucketRawPtr(b); } -static inline rax *vsetBucketRax(vsetBucket *b) { + +typedef struct vsetRaxState { + rax *r; + size_t tracked_data_bytes; /* Sum of inner bucket data sizes */ + size_t tracked_rax_overhead; /* Auto via external_logical_size */ +} vsetRaxState; + +static inline vsetRaxState *vsetBucketRaxState(vsetBucket *b) { assert(vsetBucketType(b) == VSET_BUCKET_RAX); - return (rax *)vsetBucketRawPtr(b); + return (vsetRaxState *)vsetBucketRawPtr(b); +} + +static inline rax *vsetBucketRax(vsetBucket *b) { + return vsetBucketRaxState(b)->r; } static inline void *vsetBucketSingle(vsetBucket *b) { @@ -834,7 +845,25 @@ static inline vsetBucket *vsetBucketFromNone(void) { } static inline vsetBucket *vsetBucketFromRax(rax *r) { - return vsetBucketFromRawPtr(r, VSET_BUCKET_RAX); + vsetRaxState *state = zmalloc(sizeof(*state)); + state->r = r; + state->tracked_data_bytes = 0; + state->tracked_rax_overhead = 0; + raxSetExternalLogicalSize(r, &state->tracked_rax_overhead); + return vsetBucketFromRawPtr(state, VSET_BUCKET_RAX); +} + +static inline size_t vsetInnerBucketDataSize(vsetBucket *bucket) { + switch (vsetBucketType(bucket)) { + case VSET_BUCKET_NONE: return 0; + case VSET_BUCKET_SINGLE: return 0; + case VSET_BUCKET_VECTOR: { + pVector *pv = vsetBucketVector(bucket); + return sizeof(pVector) + pvLen(pv) * sizeof(void *); + } + case VSET_BUCKET_HT: return hashtableMemUsage(vsetBucketHashtable(bucket)); + default: return 0; + } } /****************** Helper Functions *******************************************/ @@ -1108,9 +1137,12 @@ static void freeVsetBucket(vsetBucket *bucket) { case VSET_BUCKET_HT: hashtableRelease(vsetBucketHashtable(bucket)); break; - case VSET_BUCKET_RAX: - raxFreeWithCallback(vsetBucketRax(bucket), freeVsetBucket); + case VSET_BUCKET_RAX: { + vsetRaxState *state = vsetBucketRaxState(bucket); + raxFreeWithCallback(state->r, freeVsetBucket); + zfree(state); break; + } default: panic("Unknown volatile set type in freeVsetBucket"); } @@ -1127,6 +1159,8 @@ static bool splitBucketIfPossible(vsetBucket *parent, vsetGetExpiryFunc getExpir vsetBucket *new_bucket = vsetBucketFromNone(); pVector *pv = vsetBucketVector(bucket); rax *expiry_buckets = vsetBucketRax(parent); + vsetRaxState *state = vsetBucketRaxState(parent); + size_t old_size = vsetInnerBucketDataSize(bucket); /* first lets sort the vector. we cannot take a decision without it. * We set the global expiry getter so we can sort according to the provided getExpiry function. * TODO: After some thought I think it might be better to avoid sorting and attempt a quickselect. just allocate a new vector with the same size. @@ -1144,6 +1178,7 @@ static bool splitBucketIfPossible(vsetBucket *parent, vsetGetExpiryFunc getExpir assert(raxRemove(expiry_buckets, key, key_len, (void **)&new_bucket)); assert(new_bucket == bucket); target_bucket_ts = max_bucket_ts; + /* RELOCATE path: same bucket moved to a new key, zero data delta */ } else if (min_bucket_ts != max_bucket_ts) { /* lets split the bucket. we know we can do it. */ @@ -1160,6 +1195,10 @@ static bool splitBucketIfPossible(vsetBucket *parent, vsetGetExpiryFunc getExpir /* In order to avoid rax override, we directly change the node data */ // alternative: raxInsert(*set, key, key_len, bucket, NULL); raxSetData(node, bucket); + /* SPLIT path: one vector became two, track the delta */ + size_t bucket_size = vsetInnerBucketDataSize(bucket); + size_t new_bucket_size = vsetInnerBucketDataSize(new_bucket); + state->tracked_data_bytes += bucket_size + new_bucket_size - old_size; } else { /* We cannot split the bucket. just return false */ @@ -1235,8 +1274,10 @@ static inline vsetBucket *insertToBucket_RAX(vsetGetExpiryFunc getExpiry, vsetBu size_t key_len; long long bucket_ts; rax *expiry_buckets = vsetBucketRax(target); + vsetRaxState *state = vsetBucketRaxState(target); raxNode *node; vsetBucket *bucket = findBucket(expiry_buckets, expiry, key, &key_len, &bucket_ts, &node); + size_t old_size = vsetInnerBucketDataSize(bucket); int type = vsetBucketType(bucket); if (type == VSET_BUCKET_NONE) { /* No bucket: create single-entry bucket */ @@ -1244,6 +1285,7 @@ static inline vsetBucket *insertToBucket_RAX(vsetGetExpiryFunc getExpiry, vsetBu assert(vsetBucketType(bucket) == VSET_BUCKET_SINGLE); size_t key_size = encodeNewExpiryBucketKey(key, expiry); raxInsert(expiry_buckets, key, key_size, bucket, NULL); + /* SINGLE has zero data size, no delta to track */ return target; } else if (type == VSET_BUCKET_SINGLE) { /* Upgrade to vector */ @@ -1264,21 +1306,25 @@ static inline vsetBucket *insertToBucket_RAX(vsetGetExpiryFunc getExpiry, vsetBu // alternative raxInsert(expiry_buckets, key, key_len, bucket, NULL); raxSetData(node, bucket); } else { - /* we split the bucket. go and find again a bucket to place the entry since there can be new options now. */ + /* we split the bucket. go and find again a bucket to place the entry since there can be new options now. + * splitBucketIfPossible handles its own tracking, return early. */ return insertToBucket_RAX(getExpiry, target, entry, expiry); } } else { vsetBucket *new_bucket = insertToBucket_VECTOR(getExpiry, bucket, entry, expiry, -1); - if (new_bucket != bucket) + if (new_bucket != bucket) { /* In order to avoid rax override, we directly change the node data */ // alternative: raxInsert(expiry_buckets, key, key_len, new_bucket, NULL); raxSetData(node, new_bucket); + bucket = new_bucket; + } } } else if (vsetBucketType(bucket) == VSET_BUCKET_HT) { bucket = insertToBucket_HASHTABLE(getExpiry, bucket, entry, expiry); } else { panic("Unknown bucket type in insertToBucket_RAX"); } + state->tracked_data_bytes += vsetInnerBucketDataSize(bucket) - old_size; return target; } @@ -1364,6 +1410,8 @@ static inline vsetBucket *removeFromBucket_HASHTABLE(vsetGetExpiryFunc getExpiry } static bool removeEntryFromRaxBucket(vsetBucket *rax_bucket, vsetGetExpiryFunc getExpiry, void *entry, vsetBucket *bucket, unsigned char *key, size_t key_len, vsetBucket **pbucket, raxNode *node) { bool removed = false; + vsetRaxState *state = vsetBucketRaxState(rax_bucket); + size_t old_size = vsetInnerBucketDataSize(bucket); switch (vsetBucketType(bucket)) { case VSET_BUCKET_SINGLE: bucket = removeFromBucket_SINGLE(getExpiry, bucket, entry, 0, &removed); @@ -1384,6 +1432,7 @@ static bool removeEntryFromRaxBucket(vsetBucket *rax_bucket, vsetGetExpiryFunc g raxSetData(node, new_bucket); if (pbucket) *pbucket = new_bucket; } + bucket = new_bucket; } break; } @@ -1395,12 +1444,14 @@ static bool removeEntryFromRaxBucket(vsetBucket *rax_bucket, vsetGetExpiryFunc g raxSetData(node, new_bucket); if (pbucket) *pbucket = new_bucket; + bucket = new_bucket; break; } default: panic("Unknown bucket type for removeEntryFromRaxBucket"); return false; } + if (removed) state->tracked_data_bytes -= old_size - vsetInnerBucketDataSize(bucket); return removed; } @@ -1426,8 +1477,10 @@ static inline bool shrinkRaxBucketIfPossible(vsetBucket **target, vsetGetExpiryF vsetUnsetExpiryGetter(); } /* lets make our bucket to be the only left bucket */ + vsetRaxState *state = vsetBucketRaxState(*target); *target = bucket; raxFree(expiry_buckets); + zfree(state); return true; } } @@ -1517,6 +1570,7 @@ static inline size_t vsetBucketRemoveExpired_HASHTABLE(vsetBucket **bucket, vset static inline size_t vsetBucketRemoveExpired_RAX(vsetBucket **bucket, vsetGetExpiryFunc getExpiry, vsetExpiryFunc expiryFunc, mstime_t now, size_t max_count, void *ctx) { UNUSED(getExpiry); rax *buckets = vsetBucketRax(*bucket); + vsetRaxState *state = vsetBucketRaxState(*bucket); size_t count = 0; while (count < max_count && raxSize(buckets) > 0) { raxIterator it; @@ -1534,6 +1588,7 @@ static inline size_t vsetBucketRemoveExpired_RAX(vsetBucket **bucket, vsetGetExp raxStop(&it); if (time_bucket_ts > now) break; + size_t old_size = vsetInnerBucketDataSize(time_bucket); switch (time_bucket_type) { case VSET_BUCKET_SINGLE: count += vsetBucketRemoveExpired_SINGLE(&time_bucket, vsetGetExpiryZero, expiryFunc, now, max_count - count, ctx); @@ -1547,6 +1602,7 @@ static inline size_t vsetBucketRemoveExpired_RAX(vsetBucket **bucket, vsetGetExp default: panic("Cannot expire entries from bucket which is not single, vector or hashtable"); } + state->tracked_data_bytes -= old_size - vsetInnerBucketDataSize(time_bucket); if (time_bucket == VSET_NONE_BUCKET_PTR) { /* in case the bucket is freed, we can just remove it and continue to the next bucket. */ raxRemove(buckets, key, key_len, NULL); @@ -1560,6 +1616,7 @@ static inline size_t vsetBucketRemoveExpired_RAX(vsetBucket **bucket, vsetGetExp /* if all buckets are removed, */ if (raxSize(buckets) == 0) { raxFree(buckets); + zfree(state); *bucket = vsetBucketFromNone(); } else { shrinkRaxBucketIfPossible(bucket, getExpiry); @@ -1662,31 +1719,8 @@ static inline size_t vsetBucketMemUsage_HASHTABLE(vsetBucket *bucket) { } static inline size_t vsetBucketMemUsage_RAX(vsetBucket *bucket) { - rax *r = vsetBucketRax(bucket); - size_t total_mem = raxAllocSize(r); - raxIterator it; - raxStart(&it, r); - assert(raxSeek(&it, "^", NULL, 0)); - while (raxNext(&it)) { - switch (vsetBucketType(it.data)) { - case VSET_BUCKET_NONE: - total_mem += vsetBucketMemUsage_NONE(it.data); - break; - case VSET_BUCKET_SINGLE: - total_mem += vsetBucketMemUsage_SINGLE(it.data); - break; - case VSET_BUCKET_VECTOR: - total_mem += vsetBucketMemUsage_VECTOR(it.data); - break; - case VSET_BUCKET_HT: - total_mem += vsetBucketMemUsage_HASHTABLE(it.data); - break; - default: - panic("Unknown bucket type encountered in vsetBucketMemUsage_HASHTABLE"); - } - } - raxStop(&it); - return total_mem; + vsetRaxState *state = vsetBucketRaxState(bucket); + return sizeof(vsetRaxState) + state->tracked_rax_overhead + state->tracked_data_bytes; } /* Adds an entry to a volatile set (vset) based on its expiration time. @@ -1759,10 +1793,12 @@ bool vsetAddEntry(vset *set, vsetGetExpiryFunc getExpiry, void *entry) { long long max_expiry = getExpiry(pvGet(vec, len - 1)); if (get_max_bucket_ts(min_expiry) == get_max_bucket_ts(max_expiry)) { /* In case we can just insert the bucket, no need to iterate and insert it's elements. we can just push the bucket as a whole. */ + size_t existing_data = vsetInnerBucketDataSize(expiry_buckets); unsigned char key[VSET_BUCKET_KEY_LEN] = {0}; size_t key_len = encodeNewExpiryBucketKey(key, max_expiry); raxInsert(r, key, key_len, expiry_buckets, NULL); expiry_buckets = vsetBucketFromRax(r); + vsetBucketRaxState(expiry_buckets)->tracked_data_bytes = existing_data; expiry_buckets = insertToBucket_RAX(getExpiry, expiry_buckets, entry, expiry); } else { /* We need to migrate entries to the new set of buckets since we do not know all entries are in the same bucket */ @@ -2217,6 +2253,22 @@ size_t vsetMemUsage(vset *set) { return 0; } +int vsetVerifyTracking(vset *set, char *errmsg, size_t errlen) { + if (vsetBucketType(*set) != VSET_BUCKET_RAX) return 1; + vsetRaxState *state = vsetBucketRaxState(*set); + size_t walk = 0; + raxIterator it; + raxStart(&it, state->r); + raxSeek(&it, "^", NULL, 0); + while (raxNext(&it)) walk += vsetInnerBucketDataSize(it.data); + raxStop(&it); + if (state->tracked_data_bytes != walk) { + snprintf(errmsg, errlen, "vset tracked_data_bytes mismatch: tracked=%zu walk=%zu", state->tracked_data_bytes, walk); + return 0; + } + return 1; +} + /* Initializes a volatile set iterator. * * This function prepares the iterator for scanning a volatile set from the beginning. @@ -2357,7 +2409,7 @@ static size_t vsetBucketDefrag_RAX(vsetBucket **bucket, size_t cursor, void *(*d state = &defragState; state->bucket_ts = -1; state->bucket_cursor = 0; - if ((r = defragfn(r))) *bucket = vsetBucketFromRax(r); + if ((r = defragfn(r))) vsetBucketRaxState(*bucket)->r = r; r = vsetBucketRax(*bucket); } raxStart(&ri, r); @@ -2437,8 +2489,18 @@ size_t vsetScanDefrag(vset *set, size_t cursor, void *(*defragfn)(void *)) { return 0; case VSET_BUCKET_VECTOR: return vsetBucketDefrag_VECTOR(set, cursor, defragfn); - case VSET_BUCKET_RAX: + case VSET_BUCKET_RAX: { + vsetRaxState *state = vsetBucketRaxState(*set); + vsetRaxState *newstate = defragfn(state); + if (newstate) { + /* Update the rax's external pointer to the new wrapper location. + * Do NOT call raxSetExternalLogicalSize here -- it would add the + * tree size again. Just update the pointer directly. */ + newstate->r->external_logical_size = &newstate->tracked_rax_overhead; + *set = vsetBucketFromRawPtr(newstate, VSET_BUCKET_RAX); + } return vsetBucketDefrag_RAX(set, cursor, defragfn, defragRaxNode); + } default: panic("Unknown vset node type to defrag"); } diff --git a/src/vset.h b/src/vset.h index 23270395ac4..34299c3b08d 100644 --- a/src/vset.h +++ b/src/vset.h @@ -91,6 +91,7 @@ bool vsetIsValid(vset *set); long long vsetEstimatedEarliestExpiry(vset *set, vsetGetExpiryFunc getExpiry); size_t vsetRemoveExpired(vset *set, vsetGetExpiryFunc getExpiry, vsetExpiryFunc expiryFunc, mstime_t now, size_t max_count, void *ctx); size_t vsetMemUsage(vset *set); +int vsetVerifyTracking(vset *set, char *errmsg, size_t errlen); size_t vsetScanDefrag(vset *set, size_t cursor, void *(*defragfn)(void *)); #endif diff --git a/tests/unit/type/stream-tracking.tcl b/tests/unit/type/stream-tracking.tcl new file mode 100644 index 00000000000..e587708c103 --- /dev/null +++ b/tests/unit/type/stream-tracking.tcl @@ -0,0 +1,248 @@ +# Stream memory tracking integration tests. +# Uses DEBUG STREAM-VERIFY-TRACKING to validate that tracked_data_bytes +# and tracked_metadata_bytes match a full O(n) walk after each operation. + +proc verify_stream_tracking {key} { + set result [r debug stream-verify-tracking $key] + assert_equal $result "OK" +} + +start_server {tags {"stream"}} { + test {XADD tracking} { + r DEL mystream + for {set i 0} {$i < 100} {incr i} { + r XADD mystream "*" field "value_$i" + } + verify_stream_tracking mystream + } + + test {XADD with multiple fields tracking} { + r DEL mystream + for {set i 0} {$i < 50} {incr i} { + r XADD mystream "*" name "user_$i" age $i email "user_$i@test.com" + } + verify_stream_tracking mystream + } + + test {XTRIM MAXLEN tracking} { + r DEL mystream + for {set i 0} {$i < 200} {incr i} { + r XADD mystream "*" f "v_$i" + } + verify_stream_tracking mystream + r XTRIM mystream MAXLEN 10 + verify_stream_tracking mystream + } + + test {XTRIM MINID tracking} { + r DEL mystream + set ids {} + for {set i 0} {$i < 100} {incr i} { + lappend ids [r XADD mystream "*" f v] + } + verify_stream_tracking mystream + # Trim by the 50th ID + r XTRIM mystream MINID [lindex $ids 50] + verify_stream_tracking mystream + } + + test {XDEL tracking} { + r DEL mystream + set ids {} + for {set i 0} {$i < 20} {incr i} { + lappend ids [r XADD mystream "*" f "v_$i"] + } + verify_stream_tracking mystream + # Delete every other entry + for {set i 0} {$i < 20} {incr i 2} { + r XDEL mystream [lindex $ids $i] + } + verify_stream_tracking mystream + # Delete all remaining + for {set i 1} {$i < 20} {incr i 2} { + r XDEL mystream [lindex $ids $i] + } + verify_stream_tracking mystream + } + + test {XGROUP CREATE tracking} { + r DEL mystream + r XADD mystream "*" f v + r XGROUP CREATE mystream grp1 0 + verify_stream_tracking mystream + r XGROUP CREATE mystream grp2 0 + verify_stream_tracking mystream + } + + test {XGROUP DESTROY tracking} { + r DEL mystream + r XADD mystream "*" f v + r XGROUP CREATE mystream grp1 0 + r XGROUP CREATE mystream grp2 0 + verify_stream_tracking mystream + r XGROUP DESTROY mystream grp1 + verify_stream_tracking mystream + r XGROUP DESTROY mystream grp2 + verify_stream_tracking mystream + } + + test {XREADGROUP creates consumer and NACKs - tracking} { + r DEL mystream + for {set i 0} {$i < 10} {incr i} { + r XADD mystream "*" f "v_$i" + } + r XGROUP CREATE mystream grp 0 + # Read creates consumer + NACKs + r XREADGROUP GROUP grp consumer1 COUNT 5 STREAMS mystream ">" + verify_stream_tracking mystream + r XREADGROUP GROUP grp consumer2 COUNT 5 STREAMS mystream ">" + verify_stream_tracking mystream + } + + test {XACK tracking} { + r DEL mystream + set ids {} + for {set i 0} {$i < 10} {incr i} { + lappend ids [r XADD mystream "*" f "v_$i"] + } + r XGROUP CREATE mystream grp 0 + r XREADGROUP GROUP grp consumer1 COUNT 10 STREAMS mystream ">" + verify_stream_tracking mystream + # ACK half + for {set i 0} {$i < 5} {incr i} { + r XACK mystream grp [lindex $ids $i] + } + verify_stream_tracking mystream + # ACK rest + for {set i 5} {$i < 10} {incr i} { + r XACK mystream grp [lindex $ids $i] + } + verify_stream_tracking mystream + } + + test {XGROUP CREATECONSUMER and DELCONSUMER tracking} { + r DEL mystream + r XADD mystream "*" f v + r XGROUP CREATE mystream grp 0 + r XGROUP CREATECONSUMER mystream grp alice + verify_stream_tracking mystream + r XGROUP CREATECONSUMER mystream grp bob_with_longer_name + verify_stream_tracking mystream + r XGROUP DELCONSUMER mystream grp alice + verify_stream_tracking mystream + r XGROUP DELCONSUMER mystream grp bob_with_longer_name + verify_stream_tracking mystream + } + + test {XGROUP DELCONSUMER with pending NACKs tracking} { + r DEL mystream + for {set i 0} {$i < 10} {incr i} { + r XADD mystream "*" f "v_$i" + } + r XGROUP CREATE mystream grp 0 + r XREADGROUP GROUP grp myconsumer COUNT 10 STREAMS mystream ">" + verify_stream_tracking mystream + # Delete consumer with 10 pending NACKs + r XGROUP DELCONSUMER mystream grp myconsumer + verify_stream_tracking mystream + } + + test {XGROUP DESTROY with consumers and NACKs tracking} { + r DEL mystream + for {set i 0} {$i < 20} {incr i} { + r XADD mystream "*" f "v_$i" + } + r XGROUP CREATE mystream grp 0 + r XREADGROUP GROUP grp c1 COUNT 10 STREAMS mystream ">" + r XREADGROUP GROUP grp c2 COUNT 10 STREAMS mystream ">" + verify_stream_tracking mystream + # Destroy the whole group + r XGROUP DESTROY mystream grp + verify_stream_tracking mystream + } + + test {XCLAIM tracking} { + r DEL mystream + for {set i 0} {$i < 10} {incr i} { + r XADD mystream "*" f "v_$i" + } + r XGROUP CREATE mystream grp 0 + set entries [r XREADGROUP GROUP grp consumer1 COUNT 10 STREAMS mystream ">"] + verify_stream_tracking mystream + # Claim all entries to a new consumer (creates consumer2) + set ids {} + foreach entry [lindex $entries 0 1] { + lappend ids [lindex $entry 0] + } + r XCLAIM mystream grp consumer2 0 {*}$ids + verify_stream_tracking mystream + } + + test {XAUTOCLAIM tracking} { + r DEL mystream + for {set i 0} {$i < 10} {incr i} { + r XADD mystream "*" f "v_$i" + } + r XGROUP CREATE mystream grp 0 + r XREADGROUP GROUP grp old_consumer COUNT 10 STREAMS mystream ">" + verify_stream_tracking mystream + # Auto-claim to new consumer + after 10 ;# small delay so min-idle=0 works + r XAUTOCLAIM mystream grp new_consumer 0 0-0 COUNT 10 + verify_stream_tracking mystream + } + + test {XADD with MAXLEN auto-trim tracking} { + r DEL mystream + for {set i 0} {$i < 200} {incr i} { + r XADD mystream MAXLEN 50 "*" f "v_$i" + } + verify_stream_tracking mystream + } + + test {Full lifecycle tracking} { + r DEL mystream + # Add entries + for {set i 0} {$i < 50} {incr i} { + r XADD mystream "*" name "user_$i" score $i + } + verify_stream_tracking mystream + + # Create groups and consumers + r XGROUP CREATE mystream grp1 0 + r XGROUP CREATE mystream grp2 0 + verify_stream_tracking mystream + + # Deliver to consumers + r XREADGROUP GROUP grp1 alice COUNT 20 STREAMS mystream ">" + r XREADGROUP GROUP grp1 bob COUNT 20 STREAMS mystream ">" + r XREADGROUP GROUP grp2 charlie COUNT 50 STREAMS mystream ">" + verify_stream_tracking mystream + + # ACK some + set entries [r XRANGE mystream - + COUNT 10] + foreach entry $entries { + r XACK mystream grp1 [lindex $entry 0] + } + verify_stream_tracking mystream + + # Delete a consumer with pending + r XGROUP DELCONSUMER mystream grp1 bob + verify_stream_tracking mystream + + # Destroy a group + r XGROUP DESTROY mystream grp2 + verify_stream_tracking mystream + + # Trim + r XTRIM mystream MAXLEN 10 + verify_stream_tracking mystream + + # Delete remaining entries + set entries [r XRANGE mystream - +] + foreach entry $entries { + r XDEL mystream [lindex $entry 0] + } + verify_stream_tracking mystream + } +} From 83f7630719f0dfbfedf5bc9295cfe1e6f3f8ec46 Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Thu, 2 Apr 2026 14:30:51 +0000 Subject: [PATCH 2/4] reduced to 2 stream fields, fixes Signed-off-by: Lior Sventitzky --- src/rax.c | 24 ++++++++++ src/rax.h | 1 + src/stream.h | 5 +- src/t_stream.c | 81 ++++++++++++++++++--------------- src/unit/test_stream_tracking.c | 76 +++++++++++++++++-------------- 5 files changed, 112 insertions(+), 75 deletions(-) diff --git a/src/rax.c b/src/rax.c index 8b4c37c2943..141337ad4a7 100644 --- a/src/rax.c +++ b/src/rax.c @@ -551,6 +551,10 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* Otherwise set the node as a key. Note that raxSetData() * will set h->iskey. */ raxSetData(h, data); + /* raxSetData sets iskey=1/isnull=0, adding sizeof(void*) to the + * logical length. The realloc delta above was computed before the + * flags changed, so propagate the difference now. */ + raxExternalDelta(rax, sizeof(void *)); rax->numele++; return 1; /* Element inserted. */ } @@ -1050,6 +1054,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { return 0; } if (old) *old = raxGetData(h); + if (!h->isnull) raxExternalDelta(rax, -(int64_t)sizeof(void *)); h->iskey = 0; rax->numele--; @@ -1854,6 +1859,25 @@ size_t raxAllocSize(rax *rax) { return rax->alloc_size; } +/* Compute the total logical size of a rax tree by walking all nodes. + * O(n) — intended for testing/verification only. */ +static size_t raxRecursiveComputeLogicalSize(raxNode *n) { + size_t total = raxNodeCurrentLength(n); + int numchildren = n->iscompr ? 1 : n->size; + raxNode **cp = raxNodeLastChildPtr(n); + while (numchildren--) { + raxNode *child; + memcpy(&child, cp, sizeof(child)); + total += raxRecursiveComputeLogicalSize(child); + cp--; + } + return total; +} + +size_t raxComputeLogicalSize(rax *rax) { + return sizeof(*rax) + raxRecursiveComputeLogicalSize(rax->head); +} + /* ----------------------------- Introspection ------------------------------ */ /* This function is mostly used for debugging and learning purposes. diff --git a/src/rax.h b/src/rax.h index 6a8360e6761..9985b1ce251 100644 --- a/src/rax.h +++ b/src/rax.h @@ -210,6 +210,7 @@ int raxEOF(raxIterator *it); void raxShow(rax *rax); uint64_t raxSize(rax *rax); size_t raxAllocSize(rax *rax); +size_t raxComputeLogicalSize(rax *rax); void raxSetExternalLogicalSize(rax *rax, size_t *ptr); void raxFreeWithCallbackAndContext(rax *rax, void (*free_callback)(void *data, void *ctx), void *ctx); unsigned long raxTouch(raxNode *n); diff --git a/src/stream.h b/src/stream.h index 0e7516e9799..9d2b40a20fe 100644 --- a/src/stream.h +++ b/src/stream.h @@ -22,9 +22,8 @@ typedef struct stream { streamID max_deleted_entry_id; /* The maximal ID that was deleted. */ uint64_t entries_added; /* All time count of elements added. */ size_t tracked_data_bytes; /* Listpack bytes + consumer name SDS bytes. */ - size_t tracked_struct_bytes; /* sizeof(streamCG/NACK/Consumer) for all structs. */ - size_t tracked_rax_overhead; /* Auto-aggregated rax logical size across ALL - * sub-rax trees via external_logical_size pointer. */ + size_t tracked_overhead; /* Rax node overhead (auto via external_logical_size) + * + sizeof(streamCG/NACK/Consumer) for all structs. */ } stream; /* We define an iterator to iterate stream items in an abstract way, without diff --git a/src/t_stream.c b/src/t_stream.c index 71cd213bb7e..84c493755d4 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -82,9 +82,8 @@ stream *streamNew(void) { s->max_deleted_entry_id.ms = 0; s->entries_added = 0; s->tracked_data_bytes = 0; - s->tracked_struct_bytes = 0; - s->tracked_rax_overhead = 0; - raxSetExternalLogicalSize(s->rax, &s->tracked_rax_overhead); + s->tracked_overhead = 0; + raxSetExternalLogicalSize(s->rax, &s->tracked_overhead); s->cgroups = NULL; /* Created on demand to save memory when not used. */ return s; } @@ -98,16 +97,16 @@ static void streamFreeLPWithTracking(void *data, void *ctx) { void streamFreeNACKWithTracking(void *data, void *ctx) { stream *s = ctx; - s->tracked_struct_bytes -= sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamNACK); zfree(data); } void streamFreeConsumerWithTracking(void *data, void *ctx) { stream *s = ctx; streamConsumer *sc = data; - s->tracked_struct_bytes -= sizeof(streamConsumer); + s->tracked_overhead -= sizeof(streamConsumer); s->tracked_data_bytes -= sdsReqSize(sdslen(sc->name), sdsType(sc->name)); - raxFree(sc->pel); /* external pointer auto-subtracts from tracked_rax_overhead */ + raxFree(sc->pel); /* external pointer auto-subtracts from tracked_overhead */ sdsfree(sc->name); zfree(sc); } @@ -115,7 +114,7 @@ void streamFreeConsumerWithTracking(void *data, void *ctx) { void streamFreeCGWithTracking(void *data, void *ctx) { stream *s = ctx; streamCG *cg = data; - s->tracked_struct_bytes -= sizeof(streamCG); + s->tracked_overhead -= sizeof(streamCG); raxFreeWithCallbackAndContext(cg->pel, streamFreeNACKWithTracking, s); raxFreeWithCallbackAndContext(cg->consumers, streamFreeConsumerWithTracking, s); zfree(cg); @@ -131,29 +130,37 @@ void freeStream(stream *s) { /* Verify that tracked counters match a full O(n) walk. Returns 1 if correct, * 0 on mismatch with a description written to errmsg. */ int streamVerifyTracking(stream *s, char *errmsg, size_t errlen) { - size_t walk_data = 0, walk_struct = 0; + size_t walk_data = 0, walk_overhead = 0; - /* Walk all listpacks in the main rax to compute total data bytes. */ + /* Rax node overhead for all sub-rax trees. */ + walk_overhead += raxComputeLogicalSize(s->rax); + + /* Listpacks */ raxIterator ri; raxStart(&ri, s->rax); raxSeek(&ri, "^", NULL, 0); while (raxNext(&ri)) walk_data += lpBytes((unsigned char *)ri.data); raxStop(&ri); + /* CGs */ if (s->cgroups) { + walk_overhead += raxComputeLogicalSize(s->cgroups); raxStart(&ri, s->cgroups); raxSeek(&ri, "^", NULL, 0); while (raxNext(&ri)) { streamCG *cg = ri.data; - walk_struct += sizeof(streamCG); - walk_struct += raxSize(cg->pel) * sizeof(streamNACK); + walk_overhead += sizeof(streamCG); + walk_overhead += raxComputeLogicalSize(cg->pel); + walk_overhead += raxSize(cg->pel) * sizeof(streamNACK); + walk_overhead += raxComputeLogicalSize(cg->consumers); raxIterator ci; raxStart(&ci, cg->consumers); raxSeek(&ci, "^", NULL, 0); while (raxNext(&ci)) { streamConsumer *sc = ci.data; - walk_struct += sizeof(streamConsumer); + walk_overhead += sizeof(streamConsumer); + walk_overhead += raxComputeLogicalSize(sc->pel); walk_data += sdsReqSize(sdslen(sc->name), sdsType(sc->name)); } raxStop(&ci); @@ -166,9 +173,9 @@ int streamVerifyTracking(stream *s, char *errmsg, size_t errlen) { s->tracked_data_bytes, walk_data); return 0; } - if (s->tracked_struct_bytes != walk_struct) { - snprintf(errmsg, errlen, "tracked_struct_bytes mismatch: tracked=%zu walk=%zu", - s->tracked_struct_bytes, walk_struct); + if (s->tracked_overhead != walk_overhead) { + snprintf(errmsg, errlen, "tracked_overhead mismatch: tracked=%zu walk=%zu", + s->tracked_overhead, walk_overhead); return 0; } return 1; @@ -301,7 +308,7 @@ robj *streamDup(robj *o) { new_nack->delivery_time = nack->delivery_time; new_nack->delivery_count = nack->delivery_count; raxInsert(new_cg->pel, ri_cg_pel.key, sizeof(streamID), new_nack, NULL); - new_s->tracked_struct_bytes += sizeof(streamNACK); + new_s->tracked_overhead += sizeof(streamNACK); } raxStop(&ri_cg_pel); @@ -315,12 +322,12 @@ robj *streamDup(robj *o) { new_consumer = zmalloc(sizeof(*new_consumer)); new_consumer->name = sdsdup(consumer->name); new_consumer->pel = raxNew(); - raxSetExternalLogicalSize(new_consumer->pel, &new_s->tracked_rax_overhead); + raxSetExternalLogicalSize(new_consumer->pel, &new_s->tracked_overhead); raxInsert(new_cg->consumers, (unsigned char *)new_consumer->name, sdslen(new_consumer->name), new_consumer, NULL); new_consumer->seen_time = consumer->seen_time; new_consumer->active_time = consumer->active_time; - new_s->tracked_struct_bytes += sizeof(streamConsumer); + new_s->tracked_overhead += sizeof(streamConsumer); new_s->tracked_data_bytes += sdsReqSize(sdslen(new_consumer->name), sdsType(new_consumer->name)); /* Consumer PEL */ @@ -1882,7 +1889,7 @@ size_t streamReplyWithRange(client *c, * group_inserted == 0 we reused an existing NACK so no new * struct was allocated. */ if (group_inserted == 1) { - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); } consumer->active_time = commandTimeSnapshot(); @@ -2476,9 +2483,9 @@ void xreadCommand(client *c) { consumer = streamCreateConsumer(groups[i], objectGetVal(consumername), c->argv[streams_arg + i], c->db->id, SCC_DEFAULT); if (consumer) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); - raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); } if (noack) streamPropagateConsumerCreation(c, spi.keyname, spi.groupname, consumer->name); } @@ -2606,19 +2613,19 @@ static void streamFreeConsumerVoid(void *sc) { streamCG *streamCreateCG(stream *s, char *name, size_t namelen, streamID *id, long long entries_read) { if (s->cgroups == NULL) { s->cgroups = raxNew(); - raxSetExternalLogicalSize(s->cgroups, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(s->cgroups, &s->tracked_overhead); } if (raxFind(s->cgroups, (unsigned char *)name, namelen, NULL)) return NULL; streamCG *cg = zmalloc(sizeof(*cg)); cg->pel = raxNew(); - raxSetExternalLogicalSize(cg->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(cg->pel, &s->tracked_overhead); cg->consumers = raxNew(); - raxSetExternalLogicalSize(cg->consumers, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(cg->consumers, &s->tracked_overhead); cg->last_id = *id; cg->entries_read = entries_read; raxInsert(s->cgroups, (unsigned char *)name, namelen, cg, NULL); - s->tracked_struct_bytes += sizeof(streamCG); + s->tracked_overhead += sizeof(streamCG); return cg; } @@ -2830,7 +2837,7 @@ void xgroupCommand(client *c) { } else if (!strcasecmp(opt, "DESTROY") && c->argc == 4) { if (cg) { raxRemove(s->cgroups, (unsigned char *)grpname, sdslen(grpname), NULL); - s->tracked_struct_bytes -= sizeof(streamCG); + s->tracked_overhead -= sizeof(streamCG); raxFreeWithCallbackAndContext(cg->pel, streamFreeNACKWithTracking, s); raxFreeWithCallbackAndContext(cg->consumers, streamFreeConsumerWithTracking, s); zfree(cg); @@ -2845,9 +2852,9 @@ void xgroupCommand(client *c) { } else if (!strcasecmp(opt, "CREATECONSUMER") && c->argc == 5) { streamConsumer *created = streamCreateConsumer(cg, objectGetVal(c->argv[4]), c->argv[2], c->db->id, SCC_DEFAULT); if (created) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(created->name), sdsType(created->name)); - raxSetExternalLogicalSize(created->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(created->pel, &s->tracked_overhead); } addReplyLongLong(c, created ? 1 : 0); } else if (!strcasecmp(opt, "DELCONSUMER") && c->argc == 5) { @@ -2857,7 +2864,7 @@ void xgroupCommand(client *c) { /* Delete the consumer and returns the number of pending messages * that were yet associated with such a consumer. */ pending = raxSize(consumer->pel); - s->tracked_struct_bytes -= sizeof(streamConsumer) + pending * sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamConsumer) + pending * sizeof(streamNACK); s->tracked_data_bytes -= sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); streamDelConsumer(cg, consumer); server.dirty++; @@ -2989,7 +2996,7 @@ void xackCommand(client *c) { raxRemove(group->pel, buf, sizeof(buf), NULL); raxRemove(nack->consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); - ((stream *)objectGetVal(o))->tracked_struct_bytes -= sizeof(streamNACK); + ((stream *)objectGetVal(o))->tracked_overhead -= sizeof(streamNACK); acknowledged++; server.dirty++; } @@ -3349,9 +3356,9 @@ void xclaimCommand(client *c) { if (consumer == NULL) { consumer = streamCreateConsumer(group, objectGetVal(c->argv[3]), c->argv[1], c->db->id, SCC_DEFAULT); if (consumer) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); - raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); } } consumer->seen_time = commandTimeSnapshot(); @@ -3380,7 +3387,7 @@ void xclaimCommand(client *c) { raxRemove(group->pel, buf, sizeof(buf), NULL); raxRemove(nack->consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); - s->tracked_struct_bytes -= sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamNACK); } continue; } @@ -3394,7 +3401,7 @@ void xclaimCommand(client *c) { /* Create the NACK. */ nack = streamCreateNACK(NULL); raxInsert(group->pel, buf, sizeof(buf), nack, NULL); - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); } if (nack != NULL) { @@ -3536,9 +3543,9 @@ void xautoclaimCommand(client *c) { if (consumer == NULL) { consumer = streamCreateConsumer(group, objectGetVal(c->argv[3]), c->argv[1], c->db->id, SCC_DEFAULT); if (consumer) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); - raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); } } consumer->seen_time = commandTimeSnapshot(); @@ -3574,7 +3581,7 @@ void xautoclaimCommand(client *c) { raxRemove(group->pel, ri.key, ri.key_len, NULL); raxRemove(nack->consumer->pel, ri.key, ri.key_len, NULL); streamFreeNACK(nack); - s->tracked_struct_bytes -= sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamNACK); /* Remember the ID for later */ deleted_ids[deleted_id_num++] = id; raxSeek(&ri, ">=", ri.key, ri.key_len); diff --git a/src/unit/test_stream_tracking.c b/src/unit/test_stream_tracking.c index c45aac58b75..02b7ba1297d 100644 --- a/src/unit/test_stream_tracking.c +++ b/src/unit/test_stream_tracking.c @@ -3,8 +3,7 @@ * All rights reserved. * SPDX-License-Identifier: BSD-3-Clause * - * Unit tests for stream tracked_data_bytes, tracked_struct_bytes, - * and tracked_rax_overhead (Approach F: external pointer). + * Unit tests for stream tracked_data_bytes and tracked_overhead. */ #include "../fmacros.h" @@ -62,21 +61,27 @@ static size_t computeDataBytesWalk(stream *s) { return total; } -static size_t computeStructBytesWalk(stream *s) { +static size_t computeOverheadWalk(stream *s) { size_t total = 0; + total += raxComputeLogicalSize(s->rax); if (s->cgroups) { + total += raxComputeLogicalSize(s->cgroups); raxIterator ri; raxStart(&ri, s->cgroups); raxSeek(&ri, "^", NULL, 0); while (raxNext(&ri)) { streamCG *cg = ri.data; total += sizeof(streamCG); + total += raxComputeLogicalSize(cg->pel); total += raxSize(cg->pel) * sizeof(streamNACK); + total += raxComputeLogicalSize(cg->consumers); raxIterator ci; raxStart(&ci, cg->consumers); raxSeek(&ci, "^", NULL, 0); while (raxNext(&ci)) { + streamConsumer *sc = ci.data; total += sizeof(streamConsumer); + total += raxComputeLogicalSize(sc->pel); } raxStop(&ci); } @@ -88,15 +93,16 @@ static size_t computeStructBytesWalk(stream *s) { #define ASSERT_STREAM_TRACKING(s) \ do { \ size_t wd = computeDataBytesWalk(s); \ - size_t ws = computeStructBytesWalk(s); \ + size_t wo = computeOverheadWalk(s); \ if ((s)->tracked_data_bytes != wd) { \ - printf("tracked_data_bytes mismatch: tracked=%zu walk=%zu\n", \ - (s)->tracked_data_bytes, wd); \ + fprintf(stderr, "tracked_data_bytes mismatch: tracked=%zu walk=%zu at %s:%d\n", \ + (s)->tracked_data_bytes, wd, __FILE__, __LINE__); \ TEST_ASSERT(0); \ } \ - if ((s)->tracked_struct_bytes != ws) { \ - printf("tracked_struct_bytes mismatch: tracked=%zu walk=%zu\n", \ - (s)->tracked_struct_bytes, ws); \ + if ((s)->tracked_overhead != wo) { \ + fprintf(stderr, "tracked_overhead mismatch: tracked=%zu walk=%zu diff=%zd at %s:%d\n", \ + (s)->tracked_overhead, wo, \ + (ssize_t)((s)->tracked_overhead - wo), __FILE__, __LINE__); \ TEST_ASSERT(0); \ } \ } while (0) @@ -278,15 +284,15 @@ int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { sds name2 = sdsnew("bob_with_longer_name"); streamConsumer *c1 = streamCreateConsumer(cg, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); if (c1) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); - raxSetExternalLogicalSize(c1->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(c1->pel, &s->tracked_overhead); } streamConsumer *c2 = streamCreateConsumer(cg, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); if (c2) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); - raxSetExternalLogicalSize(c2->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(c2->pel, &s->tracked_overhead); } ASSERT_STREAM_TRACKING(s); @@ -297,7 +303,7 @@ int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { streamEncodeID(buf, &ids[i]); raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); raxInsert(c1->pel, buf, sizeof(buf), nack, NULL); - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); @@ -310,12 +316,12 @@ int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { raxRemove(cg->pel, buf, sizeof(buf), NULL); raxRemove(c1->pel, buf, sizeof(buf), NULL); streamFreeNACK((streamNACK *)result); - s->tracked_struct_bytes -= sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); /* Delete consumer c2 (0 NACKs) — mirrors DELCONSUMER. */ - s->tracked_struct_bytes -= sizeof(streamConsumer); + s->tracked_overhead -= sizeof(streamConsumer); s->tracked_data_bytes -= sdsReqSize(sdslen(c2->name), sdsType(c2->name)); streamDelConsumer(cg, c2); ASSERT_STREAM_TRACKING(s); @@ -345,13 +351,13 @@ int test_stream_tracking_destroy_cg(int argc, char **argv, int flags) { sds name1 = sdsnew("worker_alpha"); sds name2 = sdsnew("worker_beta_longer"); streamConsumer *c1 = streamCreateConsumer(cg2, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); - raxSetExternalLogicalSize(c1->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(c1->pel, &s->tracked_overhead); streamConsumer *c2 = streamCreateConsumer(cg2, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); - raxSetExternalLogicalSize(c2->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(c2->pel, &s->tracked_overhead); streamID ids[10]; for (int i = 0; i < 10; i++) { @@ -368,20 +374,20 @@ int test_stream_tracking_destroy_cg(int argc, char **argv, int flags) { streamConsumer *target = (i < 4 ? c1 : c2); raxInsert(cg2->pel, buf, sizeof(buf), nack, NULL); raxInsert(target->pel, buf, sizeof(buf), nack, NULL); - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); /* Destroy cg2 — mirrors xgroupCommand DESTROY. */ raxRemove(s->cgroups, (unsigned char *)"grp2", 4, NULL); - s->tracked_struct_bytes -= sizeof(streamCG); + s->tracked_overhead -= sizeof(streamCG); raxFreeWithCallbackAndContext(cg2->pel, streamFreeNACKWithTracking, s); raxFreeWithCallbackAndContext(cg2->consumers, streamFreeConsumerWithTracking, s); zfree(cg2); ASSERT_STREAM_TRACKING(s); /* grp1 still exists */ - TEST_ASSERT(s->tracked_struct_bytes > 0); + TEST_ASSERT(s->tracked_overhead > 0); sdsfree(name1); sdsfree(name2); @@ -402,9 +408,9 @@ int test_stream_tracking_del_consumer(int argc, char **argv, int flags) { robj *key = createStringObject("mystream", 8); sds name = sdsnew("busy_consumer"); streamConsumer *consumer = streamCreateConsumer(cg, name, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); - raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); /* Deliver 8 NACKs — mirrors streamReplyWithRange. */ streamID ids[8]; @@ -419,13 +425,13 @@ int test_stream_tracking_del_consumer(int argc, char **argv, int flags) { streamEncodeID(buf, &ids[i]); raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); raxInsert(consumer->pel, buf, sizeof(buf), nack, NULL); - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); /* Delete consumer — mirrors xgroupCommand DELCONSUMER. */ long long pending = raxSize(consumer->pel); - s->tracked_struct_bytes -= sizeof(streamConsumer) + pending * sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamConsumer) + pending * sizeof(streamNACK); s->tracked_data_bytes -= sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); streamDelConsumer(cg, consumer); ASSERT_STREAM_TRACKING(s); @@ -454,9 +460,9 @@ int test_stream_tracking_dup(int argc, char **argv, int flags) { robj *key = createStringObject("mystream", 8); sds name = sdsnew("consumer_one"); streamConsumer *consumer = streamCreateConsumer(cg, name, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); - raxSetExternalLogicalSize(consumer->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); /* Deliver NACKs — mirrors streamReplyWithRange. */ streamID ids[5]; @@ -471,7 +477,7 @@ int test_stream_tracking_dup(int argc, char **argv, int flags) { streamEncodeID(buf, &ids[i]); raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); raxInsert(consumer->pel, buf, sizeof(buf), nack, NULL); - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); @@ -485,7 +491,7 @@ int test_stream_tracking_dup(int argc, char **argv, int flags) { ASSERT_STREAM_TRACKING(new_s); TEST_ASSERT(s->tracked_data_bytes == new_s->tracked_data_bytes); - TEST_ASSERT(s->tracked_struct_bytes == new_s->tracked_struct_bytes); + TEST_ASSERT(s->tracked_overhead == new_s->tracked_overhead); objectSetVal(orig, NULL); /* prevent double-free of s */ decrRefCount(orig); @@ -548,9 +554,9 @@ int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { streamConsumer *c = streamCreateConsumer(cgs[cg_idx], sname, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); if (c) { - s->tracked_struct_bytes += sizeof(streamConsumer); + s->tracked_overhead += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c->name), sdsType(c->name)); - raxSetExternalLogicalSize(c->pel, &s->tracked_rax_overhead); + raxSetExternalLogicalSize(c->pel, &s->tracked_overhead); } sdsfree(sname); } else if (action < 85 && id_count > 0) { @@ -570,7 +576,7 @@ int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { streamNACK *nack = streamCreateNACK(c); if (raxTryInsert(cgs[cg_idx]->pel, buf, sizeof(buf), nack, NULL)) { raxInsert(c->pel, buf, sizeof(buf), nack, NULL); - s->tracked_struct_bytes += sizeof(streamNACK); + s->tracked_overhead += sizeof(streamNACK); if (pending_count < 4096) { pending[pending_count].cg_idx = cg_idx; pending[pending_count].id = ids[idx]; @@ -593,7 +599,7 @@ int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { raxRemove(cgs[cg_idx]->pel, buf, sizeof(buf), NULL); raxRemove(nack_consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); - s->tracked_struct_bytes -= sizeof(streamNACK); + s->tracked_overhead -= sizeof(streamNACK); } pending[idx] = pending[--pending_count]; } From 35d9d1e2aa6456c7802f3ccd56cbdcc34d296c3e Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Thu, 2 Apr 2026 18:17:52 +0000 Subject: [PATCH 3/4] moved structs size to be part of data not overhead Signed-off-by: Lior Sventitzky --- src/stream.h | 7 +- src/t_stream.c | 40 +-- ...am_tracking.c => test_stream_tracking.cpp} | 293 ++++++------------ src/unit/test_vset.cpp | 55 +--- 4 files changed, 133 insertions(+), 262 deletions(-) rename src/unit/{test_stream_tracking.c => test_stream_tracking.cpp} (69%) diff --git a/src/stream.h b/src/stream.h index 9d2b40a20fe..a9454ecd67d 100644 --- a/src/stream.h +++ b/src/stream.h @@ -21,9 +21,10 @@ typedef struct stream { streamID first_id; /* The first non-tombstone entry, zero if empty. */ streamID max_deleted_entry_id; /* The maximal ID that was deleted. */ uint64_t entries_added; /* All time count of elements added. */ - size_t tracked_data_bytes; /* Listpack bytes + consumer name SDS bytes. */ - size_t tracked_overhead; /* Rax node overhead (auto via external_logical_size) - * + sizeof(streamCG/NACK/Consumer) for all structs. */ + size_t tracked_data_bytes; /* Listpack bytes + consumer name SDS bytes + + * sizeof(streamCG/NACK/Consumer) for all structs. */ + size_t tracked_overhead; /* Rax node overhead only (auto via + * external_logical_size). */ } stream; /* We define an iterator to iterate stream items in an abstract way, without diff --git a/src/t_stream.c b/src/t_stream.c index 84c493755d4..005aa5723eb 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -97,14 +97,14 @@ static void streamFreeLPWithTracking(void *data, void *ctx) { void streamFreeNACKWithTracking(void *data, void *ctx) { stream *s = ctx; - s->tracked_overhead -= sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamNACK); zfree(data); } void streamFreeConsumerWithTracking(void *data, void *ctx) { stream *s = ctx; streamConsumer *sc = data; - s->tracked_overhead -= sizeof(streamConsumer); + s->tracked_data_bytes -= sizeof(streamConsumer); s->tracked_data_bytes -= sdsReqSize(sdslen(sc->name), sdsType(sc->name)); raxFree(sc->pel); /* external pointer auto-subtracts from tracked_overhead */ sdsfree(sc->name); @@ -114,7 +114,7 @@ void streamFreeConsumerWithTracking(void *data, void *ctx) { void streamFreeCGWithTracking(void *data, void *ctx) { stream *s = ctx; streamCG *cg = data; - s->tracked_overhead -= sizeof(streamCG); + s->tracked_data_bytes -= sizeof(streamCG); raxFreeWithCallbackAndContext(cg->pel, streamFreeNACKWithTracking, s); raxFreeWithCallbackAndContext(cg->consumers, streamFreeConsumerWithTracking, s); zfree(cg); @@ -149,9 +149,9 @@ int streamVerifyTracking(stream *s, char *errmsg, size_t errlen) { raxSeek(&ri, "^", NULL, 0); while (raxNext(&ri)) { streamCG *cg = ri.data; - walk_overhead += sizeof(streamCG); + walk_data += sizeof(streamCG); walk_overhead += raxComputeLogicalSize(cg->pel); - walk_overhead += raxSize(cg->pel) * sizeof(streamNACK); + walk_data += raxSize(cg->pel) * sizeof(streamNACK); walk_overhead += raxComputeLogicalSize(cg->consumers); raxIterator ci; @@ -159,7 +159,7 @@ int streamVerifyTracking(stream *s, char *errmsg, size_t errlen) { raxSeek(&ci, "^", NULL, 0); while (raxNext(&ci)) { streamConsumer *sc = ci.data; - walk_overhead += sizeof(streamConsumer); + walk_data += sizeof(streamConsumer); walk_overhead += raxComputeLogicalSize(sc->pel); walk_data += sdsReqSize(sdslen(sc->name), sdsType(sc->name)); } @@ -308,7 +308,7 @@ robj *streamDup(robj *o) { new_nack->delivery_time = nack->delivery_time; new_nack->delivery_count = nack->delivery_count; raxInsert(new_cg->pel, ri_cg_pel.key, sizeof(streamID), new_nack, NULL); - new_s->tracked_overhead += sizeof(streamNACK); + new_s->tracked_data_bytes += sizeof(streamNACK); } raxStop(&ri_cg_pel); @@ -327,7 +327,7 @@ robj *streamDup(robj *o) { NULL); new_consumer->seen_time = consumer->seen_time; new_consumer->active_time = consumer->active_time; - new_s->tracked_overhead += sizeof(streamConsumer); + new_s->tracked_data_bytes += sizeof(streamConsumer); new_s->tracked_data_bytes += sdsReqSize(sdslen(new_consumer->name), sdsType(new_consumer->name)); /* Consumer PEL */ @@ -1889,7 +1889,7 @@ size_t streamReplyWithRange(client *c, * group_inserted == 0 we reused an existing NACK so no new * struct was allocated. */ if (group_inserted == 1) { - s->tracked_overhead += sizeof(streamNACK); + s->tracked_data_bytes += sizeof(streamNACK); } consumer->active_time = commandTimeSnapshot(); @@ -2483,7 +2483,7 @@ void xreadCommand(client *c) { consumer = streamCreateConsumer(groups[i], objectGetVal(consumername), c->argv[streams_arg + i], c->db->id, SCC_DEFAULT); if (consumer) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); } @@ -2625,7 +2625,7 @@ streamCG *streamCreateCG(stream *s, char *name, size_t namelen, streamID *id, lo cg->last_id = *id; cg->entries_read = entries_read; raxInsert(s->cgroups, (unsigned char *)name, namelen, cg, NULL); - s->tracked_overhead += sizeof(streamCG); + s->tracked_data_bytes += sizeof(streamCG); return cg; } @@ -2837,7 +2837,7 @@ void xgroupCommand(client *c) { } else if (!strcasecmp(opt, "DESTROY") && c->argc == 4) { if (cg) { raxRemove(s->cgroups, (unsigned char *)grpname, sdslen(grpname), NULL); - s->tracked_overhead -= sizeof(streamCG); + s->tracked_data_bytes -= sizeof(streamCG); raxFreeWithCallbackAndContext(cg->pel, streamFreeNACKWithTracking, s); raxFreeWithCallbackAndContext(cg->consumers, streamFreeConsumerWithTracking, s); zfree(cg); @@ -2852,7 +2852,7 @@ void xgroupCommand(client *c) { } else if (!strcasecmp(opt, "CREATECONSUMER") && c->argc == 5) { streamConsumer *created = streamCreateConsumer(cg, objectGetVal(c->argv[4]), c->argv[2], c->db->id, SCC_DEFAULT); if (created) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(created->name), sdsType(created->name)); raxSetExternalLogicalSize(created->pel, &s->tracked_overhead); } @@ -2864,7 +2864,7 @@ void xgroupCommand(client *c) { /* Delete the consumer and returns the number of pending messages * that were yet associated with such a consumer. */ pending = raxSize(consumer->pel); - s->tracked_overhead -= sizeof(streamConsumer) + pending * sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamConsumer) + pending * sizeof(streamNACK); s->tracked_data_bytes -= sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); streamDelConsumer(cg, consumer); server.dirty++; @@ -2996,7 +2996,7 @@ void xackCommand(client *c) { raxRemove(group->pel, buf, sizeof(buf), NULL); raxRemove(nack->consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); - ((stream *)objectGetVal(o))->tracked_overhead -= sizeof(streamNACK); + ((stream *)objectGetVal(o))->tracked_data_bytes -= sizeof(streamNACK); acknowledged++; server.dirty++; } @@ -3356,7 +3356,7 @@ void xclaimCommand(client *c) { if (consumer == NULL) { consumer = streamCreateConsumer(group, objectGetVal(c->argv[3]), c->argv[1], c->db->id, SCC_DEFAULT); if (consumer) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); } @@ -3387,7 +3387,7 @@ void xclaimCommand(client *c) { raxRemove(group->pel, buf, sizeof(buf), NULL); raxRemove(nack->consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); - s->tracked_overhead -= sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamNACK); } continue; } @@ -3401,7 +3401,7 @@ void xclaimCommand(client *c) { /* Create the NACK. */ nack = streamCreateNACK(NULL); raxInsert(group->pel, buf, sizeof(buf), nack, NULL); - s->tracked_overhead += sizeof(streamNACK); + s->tracked_data_bytes += sizeof(streamNACK); } if (nack != NULL) { @@ -3543,7 +3543,7 @@ void xautoclaimCommand(client *c) { if (consumer == NULL) { consumer = streamCreateConsumer(group, objectGetVal(c->argv[3]), c->argv[1], c->db->id, SCC_DEFAULT); if (consumer) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); } @@ -3581,7 +3581,7 @@ void xautoclaimCommand(client *c) { raxRemove(group->pel, ri.key, ri.key_len, NULL); raxRemove(nack->consumer->pel, ri.key, ri.key_len, NULL); streamFreeNACK(nack); - s->tracked_overhead -= sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamNACK); /* Remember the ID for later */ deleted_ids[deleted_id_num++] = id; raxSeek(&ri, ">=", ri.key, ri.key_len); diff --git a/src/unit/test_stream_tracking.c b/src/unit/test_stream_tracking.cpp similarity index 69% rename from src/unit/test_stream_tracking.c rename to src/unit/test_stream_tracking.cpp index 02b7ba1297d..ab91dd636fe 100644 --- a/src/unit/test_stream_tracking.c +++ b/src/unit/test_stream_tracking.cpp @@ -6,21 +6,21 @@ * Unit tests for stream tracked_data_bytes and tracked_overhead. */ -#include "../fmacros.h" -#include "../server.h" -#include "../stream.h" -#include "../rax.h" -#include "../listpack.h" -#include "../sds.h" -#include "test_help.h" +#include "generated_wrappers.hpp" -#include -#include -#include -#include -#include +#include +#include +#include +#include +#include + +extern "C" { +#include "listpack.h" +#include "rax.h" +#include "sds.h" +#include "server.h" +#include "stream.h" -/* ── Forward declarations for internal functions ─────────────────────── */ int streamAppendItem(stream *s, robj **argv, int64_t numfields, streamID *added_id, streamID *use_id, int seq_given); streamCG *streamCreateCG(stream *s, char *name, size_t namelen, streamID *id, long long entries_read); streamConsumer *streamCreateConsumer(streamCG *cg, sds name, robj *key, int dbid, int flags); @@ -32,8 +32,16 @@ int64_t streamTrimByLength(stream *s, long long maxlen, int approx); int64_t streamTrimByID(stream *s, streamID minid, int approx); void streamEncodeID(void *buf, streamID *id); robj *streamDup(robj *o); +size_t raxComputeLogicalSize(rax *rax); +void streamFreeNACKWithTracking(void *data, void *ctx); +void streamFreeConsumerWithTracking(void *data, void *ctx); +void streamFreeCGWithTracking(void *data, void *ctx); +void streamFreeCG(streamCG *cg); +} -/* ── Ground truth: walk everything and compute sizes ─────────────────── */ +class StreamTrackingTest : public ::testing::Test {}; + +/* ── Ground truth ────────────────────────────────────────────────────── */ static size_t computeDataBytesWalk(stream *s) { size_t total = 0; @@ -46,12 +54,15 @@ static size_t computeDataBytesWalk(stream *s) { raxStart(&ri, s->cgroups); raxSeek(&ri, "^", NULL, 0); while (raxNext(&ri)) { - streamCG *cg = ri.data; + streamCG *cg = (streamCG *)ri.data; + total += sizeof(streamCG); + total += raxSize(cg->pel) * sizeof(streamNACK); raxIterator ci; raxStart(&ci, cg->consumers); raxSeek(&ci, "^", NULL, 0); while (raxNext(&ci)) { - streamConsumer *sc = ci.data; + streamConsumer *sc = (streamConsumer *)ci.data; + total += sizeof(streamConsumer); total += sdsReqSize(sdslen(sc->name), sdsType(sc->name)); } raxStop(&ci); @@ -70,17 +81,14 @@ static size_t computeOverheadWalk(stream *s) { raxStart(&ri, s->cgroups); raxSeek(&ri, "^", NULL, 0); while (raxNext(&ri)) { - streamCG *cg = ri.data; - total += sizeof(streamCG); + streamCG *cg = (streamCG *)ri.data; total += raxComputeLogicalSize(cg->pel); - total += raxSize(cg->pel) * sizeof(streamNACK); total += raxComputeLogicalSize(cg->consumers); raxIterator ci; raxStart(&ci, cg->consumers); raxSeek(&ci, "^", NULL, 0); while (raxNext(&ci)) { - streamConsumer *sc = ci.data; - total += sizeof(streamConsumer); + streamConsumer *sc = (streamConsumer *)ci.data; total += raxComputeLogicalSize(sc->pel); } raxStop(&ci); @@ -90,21 +98,12 @@ static size_t computeOverheadWalk(stream *s) { return total; } -#define ASSERT_STREAM_TRACKING(s) \ - do { \ - size_t wd = computeDataBytesWalk(s); \ - size_t wo = computeOverheadWalk(s); \ - if ((s)->tracked_data_bytes != wd) { \ - fprintf(stderr, "tracked_data_bytes mismatch: tracked=%zu walk=%zu at %s:%d\n", \ - (s)->tracked_data_bytes, wd, __FILE__, __LINE__); \ - TEST_ASSERT(0); \ - } \ - if ((s)->tracked_overhead != wo) { \ - fprintf(stderr, "tracked_overhead mismatch: tracked=%zu walk=%zu diff=%zd at %s:%d\n", \ - (s)->tracked_overhead, wo, \ - (ssize_t)((s)->tracked_overhead - wo), __FILE__, __LINE__); \ - TEST_ASSERT(0); \ - } \ +#define ASSERT_STREAM_TRACKING(s) \ + do { \ + ASSERT_EQ((s)->tracked_data_bytes, computeDataBytesWalk(s)) \ + << "tracked_data_bytes mismatch"; \ + ASSERT_EQ((s)->tracked_overhead, computeOverheadWalk(s)) \ + << "tracked_overhead mismatch"; \ } while (0) /* ── Helpers ─────────────────────────────────────────────────────────── */ @@ -122,11 +121,9 @@ static streamID appendEntry(stream *s, const char *field, const char *value) { /* ── Tests ───────────────────────────────────────────────────────────── */ -int test_stream_tracking_append(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, AppendEntries) { stream *s = streamNew(); server.stream_node_max_entries = 10; - for (int i = 0; i < 100; i++) { char f[16], v[32]; snprintf(f, sizeof(f), "f%d", i); @@ -134,19 +131,14 @@ int test_stream_tracking_append(int argc, char **argv, int flags) { appendEntry(s, f, v); ASSERT_STREAM_TRACKING(s); } - /* With max_entries=10, 100 entries must span multiple rax nodes */ - TEST_ASSERT(raxSize(s->rax) == 10); - + ASSERT_EQ(raxSize(s->rax), 10ul); server.stream_node_max_entries = 100; freeStream(s); - return 0; } -int test_stream_tracking_trim(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, Trim) { stream *s = streamNew(); server.stream_node_max_entries = 10; - for (int i = 0; i < 100; i++) { char f[16], v[64]; snprintf(f, sizeof(f), "f%d", i); @@ -154,20 +146,15 @@ int test_stream_tracking_trim(int argc, char **argv, int flags) { appendEntry(s, f, v); } ASSERT_STREAM_TRACKING(s); - streamTrimByLength(s, 5, 0); ASSERT_STREAM_TRACKING(s); - server.stream_node_max_entries = 100; freeStream(s); - return 0; } -int test_stream_tracking_trim_by_id(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, TrimByID) { stream *s = streamNew(); server.stream_node_max_entries = 10; - streamID ids[50]; for (int i = 0; i < 50; i++) { char f[16], v[32]; @@ -176,26 +163,18 @@ int test_stream_tracking_trim_by_id(int argc, char **argv, int flags) { ids[i] = appendEntry(s, f, v); } ASSERT_STREAM_TRACKING(s); - TEST_ASSERT(raxSize(s->rax) == 5); - + ASSERT_EQ(raxSize(s->rax), 5ul); streamTrimByID(s, ids[35], 0); ASSERT_STREAM_TRACKING(s); - TEST_ASSERT(raxSize(s->rax) < 5); - + ASSERT_LT(raxSize(s->rax), 5ul); server.stream_node_max_entries = 100; freeStream(s); - return 0; } -/* Listpack stores integers in variable-width encoding: values 0-127 use - * 7-bit (1 byte), 128+ use 13-bit (2 bytes). The "valid entries" counter - * crosses 128→127 during trim, causing a 1-byte lpBytes change. */ -int test_stream_tracking_trim_encoding_boundary(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, TrimEncodingBoundary) { stream *s = streamNew(); server.stream_node_max_entries = 200; server.stream_node_max_bytes = 0; - for (int i = 0; i < 129; i++) { char f[16], v[16]; snprintf(f, sizeof(f), "f%d", i); @@ -203,24 +182,18 @@ int test_stream_tracking_trim_encoding_boundary(int argc, char **argv, int flags appendEntry(s, f, v); } ASSERT_STREAM_TRACKING(s); - TEST_ASSERT(raxSize(s->rax) == 1); - + ASSERT_EQ(raxSize(s->rax), 1ul); size_t bytes_before = s->tracked_data_bytes; streamTrimByLength(s, 127, 0); ASSERT_STREAM_TRACKING(s); - /* Counter crossed 128→127, encoding change causes 1-byte decrease */ - TEST_ASSERT(bytes_before - s->tracked_data_bytes == 1); - + ASSERT_EQ(bytes_before - s->tracked_data_bytes, 1ul); server.stream_node_max_entries = 100; server.stream_node_max_bytes = 4096; freeStream(s); - return 0; } -int test_stream_tracking_iterator_remove(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, IteratorRemoveAll) { stream *s = streamNew(); - streamID ids[5]; for (int i = 0; i < 5; i++) { char f[16], v[16]; @@ -228,7 +201,6 @@ int test_stream_tracking_iterator_remove(int argc, char **argv, int flags) { snprintf(v, sizeof(v), "v%d", i); ids[i] = appendEntry(s, f, v); } - for (int i = 0; i < 5; i++) { streamIterator si; streamIteratorStart(&si, s, &ids[i], &ids[i], 0); @@ -240,31 +212,23 @@ int test_stream_tracking_iterator_remove(int argc, char **argv, int flags) { streamIteratorStop(&si); ASSERT_STREAM_TRACKING(s); } - TEST_ASSERT(s->tracked_data_bytes == 0); - + ASSERT_EQ(s->tracked_data_bytes, 0ul); freeStream(s); - return 0; } -int test_stream_tracking_cg_create(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, ConsumerGroupCreate) { stream *s = streamNew(); appendEntry(s, "f", "v"); - streamID zero = {0, 0}; streamCreateCG(s, (char *)"grp1", 4, &zero, 0); streamCreateCG(s, (char *)"grp2", 4, &zero, 0); ASSERT_STREAM_TRACKING(s); - freeStream(s); - return 0; } -int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, FullLifecycle) { stream *s = streamNew(); ASSERT_STREAM_TRACKING(s); - streamID ids[30]; for (int i = 0; i < 30; i++) { char f[16], v[32]; @@ -273,29 +237,26 @@ int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { ids[i] = appendEntry(s, f, v); } ASSERT_STREAM_TRACKING(s); - streamID zero = {0, 0}; streamCG *cg = streamCreateCG(s, (char *)"workers", 7, &zero, 0); ASSERT_STREAM_TRACKING(s); - /* Create consumers — mirrors command handler tracking. */ robj *key = createStringObject("mystream", 8); sds name1 = sdsnew("alice"); sds name2 = sdsnew("bob_with_longer_name"); streamConsumer *c1 = streamCreateConsumer(cg, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); if (c1) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); raxSetExternalLogicalSize(c1->pel, &s->tracked_overhead); } streamConsumer *c2 = streamCreateConsumer(cg, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); if (c2) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); raxSetExternalLogicalSize(c2->pel, &s->tracked_overhead); } ASSERT_STREAM_TRACKING(s); - /* Deliver NACKs — mirrors streamReplyWithRange (XREADGROUP). */ for (int i = 0; i < 10; i++) { streamNACK *nack = streamCreateNACK(c1); @@ -303,10 +264,9 @@ int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { streamEncodeID(buf, &ids[i]); raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); raxInsert(c1->pel, buf, sizeof(buf), nack, NULL); - s->tracked_overhead += sizeof(streamNACK); + s->tracked_data_bytes += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); - /* ACK 5 NACKs — mirrors xackCommand. */ for (int i = 0; i < 5; i++) { unsigned char buf[sizeof(streamID)]; @@ -316,49 +276,39 @@ int test_stream_tracking_full_lifecycle(int argc, char **argv, int flags) { raxRemove(cg->pel, buf, sizeof(buf), NULL); raxRemove(c1->pel, buf, sizeof(buf), NULL); streamFreeNACK((streamNACK *)result); - s->tracked_overhead -= sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); - /* Delete consumer c2 (0 NACKs) — mirrors DELCONSUMER. */ - s->tracked_overhead -= sizeof(streamConsumer); + s->tracked_data_bytes -= sizeof(streamConsumer); s->tracked_data_bytes -= sdsReqSize(sdslen(c2->name), sdsType(c2->name)); streamDelConsumer(cg, c2); ASSERT_STREAM_TRACKING(s); - - /* Trim */ streamTrimByLength(s, 10, 0); ASSERT_STREAM_TRACKING(s); - sdsfree(name1); sdsfree(name2); decrRefCount(key); freeStream(s); - return 0; } -int test_stream_tracking_destroy_cg(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, DestroyConsumerGroup) { stream *s = streamNew(); appendEntry(s, "f", "v"); - streamID zero = {0, 0}; streamCreateCG(s, (char *)"grp1", 4, &zero, 0); streamCG *cg2 = streamCreateCG(s, (char *)"grp2", 4, &zero, 0); - - /* Create consumers — mirrors command handler tracking. */ robj *key = createStringObject("mystream", 8); sds name1 = sdsnew("worker_alpha"); sds name2 = sdsnew("worker_beta_longer"); streamConsumer *c1 = streamCreateConsumer(cg2, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); raxSetExternalLogicalSize(c1->pel, &s->tracked_overhead); streamConsumer *c2 = streamCreateConsumer(cg2, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); raxSetExternalLogicalSize(c2->pel, &s->tracked_overhead); - streamID ids[10]; for (int i = 0; i < 10; i++) { char f[16], v[16]; @@ -366,7 +316,6 @@ int test_stream_tracking_destroy_cg(int argc, char **argv, int flags) { snprintf(v, sizeof(v), "v%d", i); ids[i] = appendEntry(s, f, v); } - /* Deliver NACKs — mirrors streamReplyWithRange. */ for (int i = 0; i < 7; i++) { streamNACK *nack = streamCreateNACK(i < 4 ? c1 : c2); unsigned char buf[sizeof(streamID)]; @@ -374,144 +323,112 @@ int test_stream_tracking_destroy_cg(int argc, char **argv, int flags) { streamConsumer *target = (i < 4 ? c1 : c2); raxInsert(cg2->pel, buf, sizeof(buf), nack, NULL); raxInsert(target->pel, buf, sizeof(buf), nack, NULL); - s->tracked_overhead += sizeof(streamNACK); + s->tracked_data_bytes += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); - /* Destroy cg2 — mirrors xgroupCommand DESTROY. */ raxRemove(s->cgroups, (unsigned char *)"grp2", 4, NULL); - s->tracked_overhead -= sizeof(streamCG); + s->tracked_data_bytes -= sizeof(streamCG); raxFreeWithCallbackAndContext(cg2->pel, streamFreeNACKWithTracking, s); raxFreeWithCallbackAndContext(cg2->consumers, streamFreeConsumerWithTracking, s); zfree(cg2); ASSERT_STREAM_TRACKING(s); - - /* grp1 still exists */ - TEST_ASSERT(s->tracked_overhead > 0); - + ASSERT_GT(s->tracked_overhead, 0ul); sdsfree(name1); sdsfree(name2); decrRefCount(key); freeStream(s); - return 0; } -int test_stream_tracking_del_consumer(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, DelConsumerWithNACKs) { stream *s = streamNew(); appendEntry(s, "f", "v"); - streamID zero = {0, 0}; streamCG *cg = streamCreateCG(s, (char *)"grp", 3, &zero, 0); - - /* Create consumer — mirrors command handler tracking. */ robj *key = createStringObject("mystream", 8); sds name = sdsnew("busy_consumer"); streamConsumer *consumer = streamCreateConsumer(cg, name, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); - - /* Deliver 8 NACKs — mirrors streamReplyWithRange. */ streamID ids[8]; for (int i = 0; i < 8; i++) { char f[16], v[16]; snprintf(f, sizeof(f), "f%d", i); snprintf(v, sizeof(v), "v%d", i); ids[i] = appendEntry(s, f, v); - streamNACK *nack = streamCreateNACK(consumer); unsigned char buf[sizeof(streamID)]; streamEncodeID(buf, &ids[i]); raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); raxInsert(consumer->pel, buf, sizeof(buf), nack, NULL); - s->tracked_overhead += sizeof(streamNACK); + s->tracked_data_bytes += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); - /* Delete consumer — mirrors xgroupCommand DELCONSUMER. */ long long pending = raxSize(consumer->pel); - s->tracked_overhead -= sizeof(streamConsumer) + pending * sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamConsumer) + pending * sizeof(streamNACK); s->tracked_data_bytes -= sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); streamDelConsumer(cg, consumer); ASSERT_STREAM_TRACKING(s); - sdsfree(name); decrRefCount(key); freeStream(s); - return 0; } -int test_stream_tracking_dup(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(StreamTrackingTest, StreamDup) { stream *s = streamNew(); - for (int i = 0; i < 20; i++) { char f[16], v[32]; snprintf(f, sizeof(f), "f%d", i); snprintf(v, sizeof(v), "value_%d", i); appendEntry(s, f, v); } - streamID zero = {0, 0}; streamCG *cg = streamCreateCG(s, (char *)"grp", 3, &zero, 0); - - /* Create consumer — mirrors command handler tracking. */ robj *key = createStringObject("mystream", 8); sds name = sdsnew("consumer_one"); streamConsumer *consumer = streamCreateConsumer(cg, name, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(consumer->name), sdsType(consumer->name)); raxSetExternalLogicalSize(consumer->pel, &s->tracked_overhead); - - /* Deliver NACKs — mirrors streamReplyWithRange. */ streamID ids[5]; for (int i = 0; i < 5; i++) { char f[16], v[16]; snprintf(f, sizeof(f), "nf%d", i); snprintf(v, sizeof(v), "nv%d", i); ids[i] = appendEntry(s, f, v); - streamNACK *nack = streamCreateNACK(consumer); unsigned char buf[sizeof(streamID)]; streamEncodeID(buf, &ids[i]); raxInsert(cg->pel, buf, sizeof(buf), nack, NULL); raxInsert(consumer->pel, buf, sizeof(buf), nack, NULL); - s->tracked_overhead += sizeof(streamNACK); + s->tracked_data_bytes += sizeof(streamNACK); } ASSERT_STREAM_TRACKING(s); - - /* Duplicate */ robj *orig = createStreamObject(); - freeStream(objectGetVal(orig)); + freeStream(static_cast(objectGetVal(orig))); objectSetVal(orig, s); - robj *copy = streamDup(orig); - stream *new_s = objectGetVal(copy); - + stream *new_s = (stream *)objectGetVal(copy); ASSERT_STREAM_TRACKING(new_s); - TEST_ASSERT(s->tracked_data_bytes == new_s->tracked_data_bytes); - TEST_ASSERT(s->tracked_overhead == new_s->tracked_overhead); - - objectSetVal(orig, NULL); /* prevent double-free of s */ + ASSERT_EQ(s->tracked_data_bytes, new_s->tracked_data_bytes); + ASSERT_EQ(s->tracked_overhead, new_s->tracked_overhead); + objectSetVal(orig, NULL); decrRefCount(orig); decrRefCount(copy); sdsfree(name); decrRefCount(key); freeStream(s); - return 0; } -int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); - unsigned seed = (unsigned)time(NULL) ^ (unsigned)getpid(); +TEST_F(StreamTrackingTest, Fuzzer) { + unsigned seed = static_cast(time(nullptr)) ^ static_cast(getpid()); srand(seed); printf(" Fuzzer seed: %u\n", seed); - stream *s = streamNew(); robj *key = createStringObject("mystream", 8); streamID zero = {0, 0}; - streamCG *cgs[3]; for (int i = 0; i < 3; i++) { char cgname[16]; @@ -519,32 +436,22 @@ int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { cgs[i] = streamCreateCG(s, cgname, strlen(cgname), &zero, 0); } ASSERT_STREAM_TRACKING(s); - - streamID ids[2048]; - int id_count = 0; - typedef struct { int cg_idx; streamID id; } pending_t; - pending_t pending[4096]; - int pending_count = 0; - + std::vector ids; + std::vector> pending; const int NUM_OPS = 2000; for (int op = 0; op < NUM_OPS; op++) { int action = rand() % 100; - - if (action < 40 || id_count == 0) { - /* XADD */ - if (id_count < 2048) { + if (action < 40 || ids.empty()) { + if (ids.size() < 2048) { char f[32]; snprintf(f, sizeof(f), "field_%d", op); - size_t vlen = 5 + ((size_t)rand() % 50); - char val[64]; - memset(val, 'a' + (rand() % 26), vlen); - val[vlen] = '\0'; - ids[id_count] = appendEntry(s, f, val); - id_count++; + size_t vlen = 5 + (static_cast(rand()) % 50); + std::string val(vlen, 'a' + (rand() % 26)); + streamID id = appendEntry(s, f, val.c_str()); + ids.push_back(id); } - } else if (action < 55 && id_count > 20) { - /* XTRIM */ - streamTrimByLength(s, id_count / 2, 0); + } else if (action < 55 && ids.size() > 20) { + streamTrimByLength(s, ids.size() / 2, 0); } else if (action < 70) { /* Create consumer — mirrors command handler tracking. */ int cg_idx = rand() % 3; @@ -554,12 +461,12 @@ int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { streamConsumer *c = streamCreateConsumer(cgs[cg_idx], sname, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); if (c) { - s->tracked_overhead += sizeof(streamConsumer); + s->tracked_data_bytes += sizeof(streamConsumer); s->tracked_data_bytes += sdsReqSize(sdslen(c->name), sdsType(c->name)); raxSetExternalLogicalSize(c->pel, &s->tracked_overhead); } sdsfree(sname); - } else if (action < 85 && id_count > 0) { + } else if (action < 85 && !ids.empty()) { /* Deliver NACK — mirrors streamReplyWithRange. */ int cg_idx = rand() % 3; if (raxSize(cgs[cg_idx]->consumers) > 0) { @@ -567,48 +474,40 @@ int test_stream_tracking_fuzzer(int argc, char **argv, int flags) { raxStart(&ci, cgs[cg_idx]->consumers); raxSeek(&ci, "^", NULL, 0); raxNext(&ci); - streamConsumer *c = ci.data; + streamConsumer *c = (streamConsumer *)ci.data; raxStop(&ci); - - int idx = rand() % id_count; + int idx = rand() % ids.size(); unsigned char buf[sizeof(streamID)]; streamEncodeID(buf, &ids[idx]); streamNACK *nack = streamCreateNACK(c); if (raxTryInsert(cgs[cg_idx]->pel, buf, sizeof(buf), nack, NULL)) { raxInsert(c->pel, buf, sizeof(buf), nack, NULL); - s->tracked_overhead += sizeof(streamNACK); - if (pending_count < 4096) { - pending[pending_count].cg_idx = cg_idx; - pending[pending_count].id = ids[idx]; - pending_count++; - } + s->tracked_data_bytes += sizeof(streamNACK); + pending.push_back({cg_idx, ids[idx]}); } else { streamFreeNACK(nack); } } - } else if (pending_count > 0) { + } else if (!pending.empty()) { /* ACK — mirrors xackCommand. */ - int idx = rand() % pending_count; - int cg_idx = pending[idx].cg_idx; + int idx = rand() % pending.size(); + auto [cg_idx, ack_id] = pending[idx]; unsigned char buf[sizeof(streamID)]; - streamEncodeID(buf, &pending[idx].id); + streamEncodeID(buf, &ack_id); void *result; if (raxFind(cgs[cg_idx]->pel, buf, sizeof(buf), &result)) { - streamNACK *nack = result; + streamNACK *nack = (streamNACK *)result; streamConsumer *nack_consumer = nack->consumer; raxRemove(cgs[cg_idx]->pel, buf, sizeof(buf), NULL); raxRemove(nack_consumer->pel, buf, sizeof(buf), NULL); streamFreeNACK(nack); - s->tracked_overhead -= sizeof(streamNACK); + s->tracked_data_bytes -= sizeof(streamNACK); } - pending[idx] = pending[--pending_count]; + pending.erase(pending.begin() + idx); } - if (op % 50 == 0) ASSERT_STREAM_TRACKING(s); } ASSERT_STREAM_TRACKING(s); - decrRefCount(key); freeStream(s); - return 0; } diff --git a/src/unit/test_vset.cpp b/src/unit/test_vset.cpp index ccc0b34dc65..62e6c23068c 100644 --- a/src/unit/test_vset.cpp +++ b/src/unit/test_vset.cpp @@ -485,10 +485,8 @@ TEST_F(VsetTest, TestVsetFuzzer) { #define ASSERT_VSET_TRACKING(set) \ do { \ char errmsg[256]; \ - if (!vsetVerifyTracking(set, errmsg, sizeof(errmsg))) { \ - TEST_PRINT_ERROR(errmsg); \ - return 1; \ - } \ + ASSERT_TRUE(vsetVerifyTracking(set, errmsg, sizeof(errmsg))) \ + << errmsg; \ } while (0) /* Force promotion to RAX by inserting enough entries with spread expiries */ @@ -498,8 +496,7 @@ static void fillToRax(vset *set, int count, long long base_expiry) { } } -int test_vset_tracking_add_to_rax(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingAddToRax) { vset set; vsetInit(&set); @@ -512,12 +509,9 @@ int test_vset_tracking_add_to_rax(int argc, char **argv, int flags) { } vsetClear(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_remove(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingRemoveFromRax) { vset set; vsetInit(&set); @@ -530,12 +524,9 @@ int test_vset_tracking_remove(int argc, char **argv, int flags) { } vsetRelease(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_expire(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingExpireFromRax) { vset set; vsetInit(&set); @@ -549,12 +540,9 @@ int test_vset_tracking_expire(int argc, char **argv, int flags) { expire_mock_entries(&set, LONG_LONG_MAX); vsetRelease(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_update(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingUpdateInRax) { vset set; vsetInit(&set); @@ -567,12 +555,9 @@ int test_vset_tracking_update(int argc, char **argv, int flags) { } vsetClear(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_same_bucket_promotion(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingSameBucketPromotion) { vset set; vsetInit(&set); @@ -588,12 +573,9 @@ int test_vset_tracking_same_bucket_promotion(int argc, char **argv, int flags) { } vsetClear(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_vector_to_hashtable(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingVectorToHashtable) { vset set; vsetInit(&set); @@ -610,19 +592,16 @@ int test_vset_tracking_vector_to_hashtable(int argc, char **argv, int flags) { } vsetClear(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_defrag(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingDefrag) { vset set; vsetInit(&set); fillToRax(&set, 200, 1000); ASSERT_VSET_TRACKING(&set); - TEST_ASSERT(defrag_vset(&set, 0, 0) == 0); + ASSERT_EQ(defrag_vset(&set, 0, 0), 0u); ASSERT_VSET_TRACKING(&set); for (int i = 0; i < 50; i++) { @@ -631,12 +610,9 @@ int test_vset_tracking_defrag(int argc, char **argv, int flags) { ASSERT_VSET_TRACKING(&set); vsetClear(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_shrink(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); +TEST_F(VsetTest, TrackingShrinkFromRax) { vset set; vsetInit(&set); @@ -649,13 +625,10 @@ int test_vset_tracking_shrink(int argc, char **argv, int flags) { } vsetRelease(&set); - free_mock_entries(); - return 0; } -int test_vset_tracking_fuzzer(int argc, char **argv, int flags) { - UNUSED(argc); UNUSED(argv); UNUSED(flags); - unsigned seed = (unsigned)time(NULL) ^ (unsigned)getpid(); +TEST_F(VsetTest, TrackingFuzzer) { + unsigned seed = static_cast(time(nullptr)) ^ static_cast(getpid()); srand(seed); printf(" Vset tracking fuzzer seed: %u\n", seed); @@ -690,6 +663,4 @@ int test_vset_tracking_fuzzer(int argc, char **argv, int flags) { expire_mock_entries(&set, LONG_LONG_MAX); vsetRelease(&set); - free_mock_entries(); - return 0; } From 987caeb6651afd599f83eea3052a842df5ac1ea5 Mon Sep 17 00:00:00 2001 From: Lior Sventitzky Date: Fri, 3 Apr 2026 18:50:52 +0000 Subject: [PATCH 4/4] minor fixes Signed-off-by: Lior Sventitzky minor fixes Signed-off-by: Lior Sventitzky --- src/rax.c | 45 ++++++++++++----------- src/rax.h | 8 ++--- src/t_stream.c | 3 +- src/unit/test_stream_tracking.cpp | 55 +++++++++++++++++++++++++---- src/unit/test_vset.cpp | 14 ++++---- src/vset.c | 38 ++++++++++++++++++-- tests/unit/type/stream-tracking.tcl | 2 +- 7 files changed, 118 insertions(+), 47 deletions(-) diff --git a/src/rax.c b/src/rax.c index 141337ad4a7..8c1e539f992 100644 --- a/src/rax.c +++ b/src/rax.c @@ -212,7 +212,7 @@ void raxSetExternalLogicalSize(rax *rax, size_t *ptr) { } /* Propagate a logical size delta to the external counter, if set. */ -static inline void raxExternalDelta(rax *rax, int64_t delta) { +static inline void raxExternalOverheadDelta(rax *rax, int64_t delta) { if (rax->external_logical_size) *rax->external_logical_size += delta; } @@ -532,7 +532,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** if (h) { memcpy(parentlink, &h, sizeof(h)); rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(h); - raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(h) - oldlogical)); + raxExternalOverheadDelta(rax, (int64_t)(raxNodeCurrentLength(h) - oldlogical)); } } if (h == NULL) { @@ -554,7 +554,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* raxSetData sets iskey=1/isnull=0, adding sizeof(void*) to the * logical length. The realloc delta above was computed before the * flags changed, so propagate the difference now. */ - raxExternalDelta(rax, sizeof(void *)); + raxExternalOverheadDelta(rax, sizeof(void *)); rax->numele++; return 1; /* Element inserted. */ } @@ -733,7 +733,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** } splitnode->data[0] = h->data[j]; rax->alloc_size += rax_ptr_alloc_size(splitnode); - raxExternalDelta(rax, raxNodeCurrentLength(splitnode)); + raxExternalOverheadDelta(rax, raxNodeCurrentLength(splitnode)); if (j == 0) { /* 3a: Replace the old node with the split node. */ @@ -759,7 +759,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** parentlink = cp; /* Set parentlink to splitnode parent. */ rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(trimmed); - raxExternalDelta(rax, raxNodeCurrentLength(trimmed)); + raxExternalOverheadDelta(rax, raxNodeCurrentLength(trimmed)); } /* 4: Create the postfix node: what remains of the original @@ -775,7 +775,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** memcpy(cp, &next, sizeof(next)); rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(postfix); - raxExternalDelta(rax, raxNodeCurrentLength(postfix)); + raxExternalOverheadDelta(rax, raxNodeCurrentLength(postfix)); } else { /* 4b: just use next as postfix node. */ postfix = next; @@ -788,7 +788,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* 6. Continue insertion: this will cause the splitnode to * get a new child (the non common character at the currently * inserted key). */ - raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(h)); + raxExternalOverheadDelta(rax, -(int64_t)raxNodeCurrentLength(h)); rax->alloc_size -= rax_ptr_alloc_size(h); rax_free(h); h = splitnode; @@ -829,7 +829,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** memcpy(cp, &next, sizeof(next)); rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(postfix); - raxExternalDelta(rax, raxNodeCurrentLength(postfix)); + raxExternalOverheadDelta(rax, raxNodeCurrentLength(postfix)); /* 3: Trim the compressed node. */ trimmed->size = j; @@ -843,7 +843,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** raxSetData(trimmed, aux); } rax->alloc_size += rax_ptr_alloc_size(trimmed); - raxExternalDelta(rax, raxNodeCurrentLength(trimmed)); + raxExternalOverheadDelta(rax, raxNodeCurrentLength(trimmed)); /* Fix the trimmed node child pointer to point to * the postfix node. */ @@ -853,7 +853,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** /* Finish! We don't need to continue with the insertion * algorithm for ALGO 2. The key is already inserted. */ rax->numele++; - raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(h)); + raxExternalOverheadDelta(rax, -(int64_t)raxNodeCurrentLength(h)); rax->alloc_size -= rax_ptr_alloc_size(h); rax_free(h); return 1; /* Key inserted. */ @@ -891,7 +891,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** } rax->numnodes++; rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(h) + rax_ptr_alloc_size(child); - raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(h) + raxNodeCurrentLength(child) - oldlogical)); + raxExternalOverheadDelta(rax, (int64_t)(raxNodeCurrentLength(h) + raxNodeCurrentLength(child) - oldlogical)); h = child; } size_t oldalloc = rax_ptr_alloc_size(h); @@ -903,7 +903,7 @@ int raxGenericInsert(rax *rax, unsigned char *s, size_t len, void *data, void ** raxSetData(h, data); memcpy(parentlink, &h, sizeof(h)); rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(h); - raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(h) - oldlogical)); + raxExternalOverheadDelta(rax, (int64_t)(raxNodeCurrentLength(h) - oldlogical)); return 1; /* Element inserted. */ oom: @@ -1054,7 +1054,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { return 0; } if (old) *old = raxGetData(h); - if (!h->isnull) raxExternalDelta(rax, -(int64_t)sizeof(void *)); + if (!h->isnull) raxExternalOverheadDelta(rax, -(int64_t)sizeof(void *)); h->iskey = 0; rax->numele--; @@ -1074,7 +1074,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { child = h; debugf("Freeing child %p [%.*s] key:%d\n", (void *)child, (int)child->size, (char *)child->data, child->iskey); - raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(child)); + raxExternalOverheadDelta(rax, -(int64_t)raxNodeCurrentLength(child)); rax->alloc_size -= rax_ptr_alloc_size(child); rax_free(child); rax->numnodes--; @@ -1089,7 +1089,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { size_t oldlogical = raxNodeCurrentLength(h); raxNode *new = raxRemoveChild(h, child); rax->alloc_size = rax->alloc_size - oldalloc + rax_ptr_alloc_size(new); - raxExternalDelta(rax, (int64_t)(raxNodeCurrentLength(new) - oldlogical)); + raxExternalOverheadDelta(rax, (int64_t)(raxNodeCurrentLength(new) - oldlogical)); if (new != h) { raxNode *parent = raxStackPeek(&ts); raxNode **parentlink; @@ -1207,7 +1207,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { new->size = comprsize; rax->numnodes++; rax->alloc_size += rax_ptr_alloc_size(new); - raxExternalDelta(rax, raxNodeCurrentLength(new)); + raxExternalOverheadDelta(rax, raxNodeCurrentLength(new)); /* Scan again, this time to populate the new node content and * to fix the new node child pointer. At the same time we free @@ -1220,7 +1220,7 @@ int raxRemove(rax *rax, unsigned char *s, size_t len, void **old) { raxNode **cp = raxNodeLastChildPtr(h); raxNode *tofree = h; memcpy(&h, cp, sizeof(h)); - raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(tofree)); + raxExternalOverheadDelta(rax, -(int64_t)raxNodeCurrentLength(tofree)); rax->alloc_size -= rax_ptr_alloc_size(tofree); rax_free(tofree); rax->numnodes--; @@ -1263,7 +1263,7 @@ void raxRecursiveFree(rax *rax, raxNode *n, void (*free_callback)(void *)) { } debugnode("free depth-first", n); if (free_callback && n->iskey && !n->isnull) free_callback(raxGetData(n)); - raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(n)); + raxExternalOverheadDelta(rax, -(int64_t)raxNodeCurrentLength(n)); rax_free(n); rax->numnodes--; } @@ -1273,13 +1273,12 @@ void raxRecursiveFree(rax *rax, raxNode *n, void (*free_callback)(void *)) { void raxFreeWithCallback(rax *rax, void (*free_callback)(void *)) { raxRecursiveFree(rax, rax->head, free_callback); assert(rax->numnodes == 0); - raxExternalDelta(rax, -(int64_t)sizeof(*rax)); + raxExternalOverheadDelta(rax, -(int64_t)sizeof(*rax)); rax_free(rax); } /* Same as raxRecursiveFree but the callback receives a context pointer. */ -static void raxRecursiveFreeWithContext(rax *rax, raxNode *n, - void (*free_callback)(void *data, void *ctx), void *ctx) { +static void raxRecursiveFreeWithContext(rax *rax, raxNode *n, void (*free_callback)(void *data, void *ctx), void *ctx) { debugnode("free traversing", n); int numchildren = n->iscompr ? 1 : n->size; raxNode **cp = raxNodeLastChildPtr(n); @@ -1291,7 +1290,7 @@ static void raxRecursiveFreeWithContext(rax *rax, raxNode *n, } debugnode("free depth-first", n); if (free_callback && n->iskey && !n->isnull) free_callback(raxGetData(n), ctx); - raxExternalDelta(rax, -(int64_t)raxNodeCurrentLength(n)); + raxExternalOverheadDelta(rax, -(int64_t)raxNodeCurrentLength(n)); rax_free(n); rax->numnodes--; } @@ -1301,7 +1300,7 @@ static void raxRecursiveFreeWithContext(rax *rax, raxNode *n, void raxFreeWithCallbackAndContext(rax *rax, void (*free_callback)(void *data, void *ctx), void *ctx) { raxRecursiveFreeWithContext(rax, rax->head, free_callback, ctx); assert(rax->numnodes == 0); - raxExternalDelta(rax, -(int64_t)sizeof(*rax)); + raxExternalOverheadDelta(rax, -(int64_t)sizeof(*rax)); rax_free(rax); } diff --git a/src/rax.h b/src/rax.h index 9985b1ce251..c67d2f56f6d 100644 --- a/src/rax.h +++ b/src/rax.h @@ -131,10 +131,10 @@ typedef struct raxNode { } raxNode; typedef struct rax { - raxNode *head; /* Pointer to root node of tree */ - uint64_t numele; /* Number of keys in the tree */ - uint64_t numnodes; /* Number of rax nodes in the tree */ - size_t alloc_size; /* Total allocation size of the tree in bytes */ + raxNode *head; /* Pointer to root node of tree */ + uint64_t numele; /* Number of keys in the tree */ + uint64_t numnodes; /* Number of rax nodes in the tree */ + size_t alloc_size; /* Total allocation size of the tree in bytes */ size_t *external_logical_size; /* If non-NULL, logical size deltas * (using raxNodeCurrentLength) are * propagated here on every mutation. diff --git a/src/t_stream.c b/src/t_stream.c index 005aa5723eb..f2fe6571b87 100644 --- a/src/t_stream.c +++ b/src/t_stream.c @@ -2659,13 +2659,12 @@ streamConsumer *streamCreateConsumer(streamCG *cg, sds name, robj *key, int dbid int notify = !(flags & SCC_NO_NOTIFY); int dirty = !(flags & SCC_NO_DIRTIFY); streamConsumer *consumer = zmalloc(sizeof(*consumer)); - consumer->name = sdsdup(name); int success = raxTryInsert(cg->consumers, (unsigned char *)name, sdslen(name), consumer, NULL); if (!success) { - sdsfree(consumer->name); zfree(consumer); return NULL; } + consumer->name = sdsdup(name); consumer->pel = raxNew(); consumer->active_time = -1; consumer->seen_time = commandTimeSnapshot(); diff --git a/src/unit/test_stream_tracking.cpp b/src/unit/test_stream_tracking.cpp index ab91dd636fe..03ceeb232cb 100644 --- a/src/unit/test_stream_tracking.cpp +++ b/src/unit/test_stream_tracking.cpp @@ -11,8 +11,8 @@ #include #include #include -#include #include +#include extern "C" { #include "listpack.h" @@ -98,12 +98,12 @@ static size_t computeOverheadWalk(stream *s) { return total; } -#define ASSERT_STREAM_TRACKING(s) \ - do { \ - ASSERT_EQ((s)->tracked_data_bytes, computeDataBytesWalk(s)) \ - << "tracked_data_bytes mismatch"; \ - ASSERT_EQ((s)->tracked_overhead, computeOverheadWalk(s)) \ - << "tracked_overhead mismatch"; \ +#define ASSERT_STREAM_TRACKING(s) \ + do { \ + ASSERT_EQ((s)->tracked_data_bytes, computeDataBytesWalk(s)) \ + << "tracked_data_bytes mismatch"; \ + ASSERT_EQ((s)->tracked_overhead, computeOverheadWalk(s)) \ + << "tracked_overhead mismatch"; \ } while (0) /* ── Helpers ─────────────────────────────────────────────────────────── */ @@ -511,3 +511,44 @@ TEST_F(StreamTrackingTest, Fuzzer) { decrRefCount(key); freeStream(s); } + +TEST_F(StreamTrackingTest, SharedPrefixConsumerRemoval) { + stream *s = streamNew(); + appendEntry(s, "f", "v"); + + streamID zero = {0, 0}; + streamCG *cg = streamCreateCG(s, (char *)"grp", 3, &zero, 0); + ASSERT_STREAM_TRACKING(s); + + robj *key = createStringObject("mystream", 8); + sds name1 = sdsnew("worker"); + sds name2 = sdsnew("worker_alpha"); + streamConsumer *c1 = streamCreateConsumer(cg, name1, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + if (c1) { + s->tracked_data_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c1->name), sdsType(c1->name)); + raxSetExternalLogicalSize(c1->pel, &s->tracked_overhead); + } + streamConsumer *c2 = streamCreateConsumer(cg, name2, key, 0, SCC_NO_NOTIFY | SCC_NO_DIRTIFY); + if (c2) { + s->tracked_data_bytes += sizeof(streamConsumer); + s->tracked_data_bytes += sdsReqSize(sdslen(c2->name), sdsType(c2->name)); + raxSetExternalLogicalSize(c2->pel, &s->tracked_overhead); + } + ASSERT_STREAM_TRACKING(s); + + s->tracked_data_bytes -= sizeof(streamConsumer); + s->tracked_data_bytes -= sdsReqSize(sdslen(c1->name), sdsType(c1->name)); + streamDelConsumer(cg, c1); + ASSERT_STREAM_TRACKING(s); + + s->tracked_data_bytes -= sizeof(streamConsumer); + s->tracked_data_bytes -= sdsReqSize(sdslen(c2->name), sdsType(c2->name)); + streamDelConsumer(cg, c2); + ASSERT_STREAM_TRACKING(s); + + sdsfree(name1); + sdsfree(name2); + decrRefCount(key); + freeStream(s); +} diff --git a/src/unit/test_vset.cpp b/src/unit/test_vset.cpp index 62e6c23068c..afe7c56c703 100644 --- a/src/unit/test_vset.cpp +++ b/src/unit/test_vset.cpp @@ -482,11 +482,11 @@ TEST_F(VsetTest, TestVsetFuzzer) { /* ── Tracking tests ─────────────────────────────────────────────────── */ -#define ASSERT_VSET_TRACKING(set) \ - do { \ - char errmsg[256]; \ - ASSERT_TRUE(vsetVerifyTracking(set, errmsg, sizeof(errmsg))) \ - << errmsg; \ +#define ASSERT_VSET_TRACKING(set) \ + do { \ + char errmsg[256]; \ + ASSERT_TRUE(vsetVerifyTracking(set, errmsg, sizeof(errmsg))) \ + << errmsg; \ } while (0) /* Force promotion to RAX by inserting enough entries with spread expiries */ @@ -538,7 +538,7 @@ TEST_F(VsetTest, TrackingExpireFromRax) { ASSERT_VSET_TRACKING(&set); } - expire_mock_entries(&set, LONG_LONG_MAX); + expire_mock_entries(&set, LLONG_MAX); vsetRelease(&set); } @@ -661,6 +661,6 @@ TEST_F(VsetTest, TrackingFuzzer) { } ASSERT_VSET_TRACKING(&set); - expire_mock_entries(&set, LONG_LONG_MAX); + expire_mock_entries(&set, LLONG_MAX); vsetRelease(&set); } diff --git a/src/vset.c b/src/vset.c index 8bafee11d4b..38eeb4fb639 100644 --- a/src/vset.c +++ b/src/vset.c @@ -803,9 +803,18 @@ static inline hashtable *vsetBucketHashtable(vsetBucket *b) { } +/* Wrapper around rax for RAX-encoded vset buckets. Holds tracking + * counters so vsetMemUsage can be O(1) instead of iterating all inner + * buckets. The tagged VSET_BUCKET_RAX pointer points to this struct. + * + * tracked_data_bytes covers only the vset container overhead (pVector + * headers + pointer arrays, hashtable bucket arrays). The actual entry + * data is owned and counted by the hash's hashtable tracking, not here. */ typedef struct vsetRaxState { rax *r; - size_t tracked_data_bytes; /* Sum of inner bucket data sizes */ + size_t tracked_data_bytes; /* Sum of inner bucket logical sizes + * (sizeof(pVector) + len*sizeof(void*) for VECTOR, + * hashtableMemUsage for HT). */ size_t tracked_rax_overhead; /* Auto via external_logical_size */ } vsetRaxState; @@ -1719,8 +1728,31 @@ static inline size_t vsetBucketMemUsage_HASHTABLE(vsetBucket *bucket) { } static inline size_t vsetBucketMemUsage_RAX(vsetBucket *bucket) { - vsetRaxState *state = vsetBucketRaxState(bucket); - return sizeof(vsetRaxState) + state->tracked_rax_overhead + state->tracked_data_bytes; + rax *r = vsetBucketRax(bucket); + size_t total_mem = raxAllocSize(r); + raxIterator it; + raxStart(&it, r); + assert(raxSeek(&it, "^", NULL, 0)); + while (raxNext(&it)) { + switch (vsetBucketType(it.data)) { + case VSET_BUCKET_NONE: + total_mem += vsetBucketMemUsage_NONE(it.data); + break; + case VSET_BUCKET_SINGLE: + total_mem += vsetBucketMemUsage_SINGLE(it.data); + break; + case VSET_BUCKET_VECTOR: + total_mem += vsetBucketMemUsage_VECTOR(it.data); + break; + case VSET_BUCKET_HT: + total_mem += vsetBucketMemUsage_HASHTABLE(it.data); + break; + default: + panic("Unknown bucket type encountered in vsetBucketMemUsage_RAX"); + } + } + raxStop(&it); + return total_mem; } /* Adds an entry to a volatile set (vset) based on its expiration time. diff --git a/tests/unit/type/stream-tracking.tcl b/tests/unit/type/stream-tracking.tcl index e587708c103..10cedebb49f 100644 --- a/tests/unit/type/stream-tracking.tcl +++ b/tests/unit/type/stream-tracking.tcl @@ -7,7 +7,7 @@ proc verify_stream_tracking {key} { assert_equal $result "OK" } -start_server {tags {"stream"}} { +start_server {tags {"stream needs:debug"}} { test {XADD tracking} { r DEL mystream for {set i 0} {$i < 100} {incr i} {