Unverified Commit 2dd4cca3 authored by debing.sun's avatar debing.sun Committed by GitHub
Browse files

Increment kvstore's non_empty_dicts only on first insert (#13528)



Found by @oranagra 

Currently, when the size of dict becomes 1, we do not check whether
`delta` is positive or negative.
As a result, `non_empty_dicts` is still incremented when the size of
dict changes from 2 to 1.
We should only increment `non_empty_dicts` when `delta` is positive, as
this indicates the first time an element is inserted into the dict.

---------
Co-authored-by: default avataroranagra <oran@redislabs.com>
parent bcae7708
...@@ -124,7 +124,9 @@ static void cumulativeKeyCountAdd(kvstore *kvs, int didx, long delta) { ...@@ -124,7 +124,9 @@ static void cumulativeKeyCountAdd(kvstore *kvs, int didx, long delta) {
dict *d = kvstoreGetDict(kvs, didx); dict *d = kvstoreGetDict(kvs, didx);
size_t dsize = dictSize(d); size_t dsize = dictSize(d);
int non_empty_dicts_delta = dsize == 1? 1 : dsize == 0? -1 : 0; /* Increment if dsize is 1 and delta is positive (first element inserted, dict becomes non-empty).
* Decrement if dsize is 0 (dict becomes empty). */
int non_empty_dicts_delta = (dsize == 1 && delta > 0) ? 1 : (dsize == 0) ? -1 : 0;
kvs->non_empty_dicts += non_empty_dicts_delta; kvs->non_empty_dicts += non_empty_dicts_delta;
/* BIT does not need to be calculated when there's only one dict. */ /* BIT does not need to be calculated when there's only one dict. */
...@@ -1026,6 +1028,31 @@ int kvstoreTest(int argc, char **argv, int flags) { ...@@ -1026,6 +1028,31 @@ int kvstoreTest(int argc, char **argv, int flags) {
kvstoreRelease(kvs); kvstoreRelease(kvs);
} }
TEST("Verify non-empty dict count is correctly updated") {
kvstore *kvs = kvstoreCreate(&KvstoreDictTestType, 2, KVSTORE_ALLOCATE_DICTS_ON_DEMAND);
for (int idx = 0; idx < 4; idx++) {
for (i = 0; i < 16; i++) {
de = kvstoreDictAddRaw(kvs, idx, stringFromInt(i), NULL);
assert(de != NULL);
/* When the first element is inserted, the number of non-empty dictionaries is increased by 1. */
if (i == 0) assert(kvstoreNumNonEmptyDicts(kvs) == idx + 1);
}
}
/* Step by step, clear all dictionaries and ensure non-empty dict count is updated */
for (int idx = 0; idx < 4; idx++) {
kvs_di = kvstoreGetDictSafeIterator(kvs, idx);
while((de = kvstoreDictIteratorNext(kvs_di)) != NULL) {
key = dictGetKey(de);
assert(kvstoreDictDelete(kvs, idx, key) == DICT_OK);
/* When the dictionary is emptied, the number of non-empty dictionaries is reduced by 1. */
if (kvstoreDictSize(kvs, idx) == 0) assert(kvstoreNumNonEmptyDicts(kvs) == 3 - idx);
}
kvstoreReleaseDictIterator(kvs_di);
}
kvstoreRelease(kvs);
}
kvstoreRelease(kvs1); kvstoreRelease(kvs1);
kvstoreRelease(kvs2); kvstoreRelease(kvs2);
return 0; return 0;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment