Unverified Commit 56169112 authored by Moti Cohen's avatar Moti Cohen Committed by GitHub
Browse files

Fix returned value nextExpireTime by ebExpire() (#13313)

At `ebuckets` structure, On `ebExpire()`, if the callback indicated to update 
the item expiration time and return it back to ebuckets (`ACT_UPDATE_EXP_ITEM`), 
then returned value `nextExpireTime` should be updated, if needed. 
Invalid value of `nextExpireTime` was modified from 0 to `EB_EXPIRE_TIME_INVALID`.
parent 50569a90
...@@ -385,6 +385,8 @@ int ebSingleSegExpire(FirstSegHdr *firstSegHdr, ...@@ -385,6 +385,8 @@ int ebSingleSegExpire(FirstSegHdr *firstSegHdr,
break; break;
} }
/* If indicated to re-insert the item, then chain it to updateList.
* it will be ebAdd() back to ebuckets at the end of ebExpire() */
if (act == ACT_UPDATE_EXP_ITEM) { if (act == ACT_UPDATE_EXP_ITEM) {
mIter->next = *updateList; mIter->next = *updateList;
*updateList = iter; *updateList = iter;
...@@ -457,6 +459,8 @@ static int ebSegExpire(FirstSegHdr *firstSegHdr, ...@@ -457,6 +459,8 @@ static int ebSegExpire(FirstSegHdr *firstSegHdr,
break; break;
} }
/* If indicated to re-insert the item, then chain it to updateList.
* it will be ebAdd() back to ebuckets at the end of ebExpire() */
if (act == ACT_UPDATE_EXP_ITEM) { if (act == ACT_UPDATE_EXP_ITEM) {
mIter->next = *updateList; mIter->next = *updateList;
*updateList = iter; *updateList = iter;
...@@ -695,6 +699,8 @@ static int ebListExpire(ebuckets *eb, ...@@ -695,6 +699,8 @@ static int ebListExpire(ebuckets *eb,
break; break;
} }
/* If indicated to re-insert the item, then chain it to updateList.
* it will be ebAdd() back to ebuckets at the end of ebExpire() */
if (act == ACT_UPDATE_EXP_ITEM) { if (act == ACT_UPDATE_EXP_ITEM) {
metaItem->next = *updateList; metaItem->next = *updateList;
*updateList = item; *updateList = item;
...@@ -707,7 +713,7 @@ static int ebListExpire(ebuckets *eb, ...@@ -707,7 +713,7 @@ static int ebListExpire(ebuckets *eb,
if (expired == numItems) { if (expired == numItems) {
*eb = NULL; *eb = NULL;
info->nextExpireTime = 0; info->nextExpireTime = EB_EXPIRE_TIME_INVALID;
return 1; return 1;
} }
...@@ -1458,11 +1464,14 @@ int ebAdd(ebuckets *eb, EbucketsType *type, eItem item, uint64_t expireTime) { ...@@ -1458,11 +1464,14 @@ int ebAdd(ebuckets *eb, EbucketsType *type, eItem item, uint64_t expireTime) {
void ebExpire(ebuckets *eb, EbucketsType *type, ExpireInfo *info) { void ebExpire(ebuckets *eb, EbucketsType *type, ExpireInfo *info) {
/* updateList - maintain a list of expired items that the callback `onExpireItem` /* updateList - maintain a list of expired items that the callback `onExpireItem`
* indicated to update their expiration time rather than removing them. * indicated to update their expiration time rather than removing them.
* At the end of this function, `updateList` will be `ebAdd()` back. */ * At the end of this function, the items will be `ebAdd()` back.
*
* Note, this list of items does not allocate any memory, but temporary reuses
* the `next` pointer of the `ExpireMeta` structure of the expired items. */
eItem updateList = NULL; eItem updateList = NULL;
/* reset info outputs */ /* reset info outputs */
info->nextExpireTime = 0; info->nextExpireTime = EB_EXPIRE_TIME_INVALID;
info->itemsExpired = 0; info->itemsExpired = 0;
/* if empty ebuckets */ /* if empty ebuckets */
...@@ -1523,7 +1532,14 @@ END_ACTEXP: ...@@ -1523,7 +1532,14 @@ END_ACTEXP:
while (updateList) { while (updateList) {
ExpireMeta *mItem = type->getExpireMeta(updateList); ExpireMeta *mItem = type->getExpireMeta(updateList);
eItem next = mItem->next; eItem next = mItem->next;
ebAdd(eb, type, updateList, ebGetMetaExpTime(mItem)); uint64_t expireAt = ebGetMetaExpTime(mItem);
/* Update next minimum expire time if needed.
* Condition is valid also if nextExpireTime is EB_EXPIRE_TIME_INVALID */
if (expireAt < info->nextExpireTime)
info->nextExpireTime = expireAt;
ebAdd(eb, type, updateList, expireAt);
updateList = next; updateList = next;
} }
...@@ -2179,7 +2195,7 @@ int ebucketsTest(int argc, char **argv, int flags) { ...@@ -2179,7 +2195,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
assert(info.itemsExpired == 1); assert(info.itemsExpired == 1);
if (i == numItems) { /* if last item */ if (i == numItems) { /* if last item */
assert(eb == NULL); assert(eb == NULL);
assert(info.nextExpireTime == 0); assert(info.nextExpireTime == EB_EXPIRE_TIME_INVALID);
} else { } else {
assert(info.nextExpireTime == EB_BUCKET_EXP_TIME(i)); assert(info.nextExpireTime == EB_BUCKET_EXP_TIME(i));
} }
...@@ -2209,7 +2225,7 @@ int ebucketsTest(int argc, char **argv, int flags) { ...@@ -2209,7 +2225,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
assert(info.itemsExpired == expirePerIter); assert(info.itemsExpired == expirePerIter);
if (i == numIterations) { /* if last item */ if (i == numIterations) { /* if last item */
assert(eb == NULL); assert(eb == NULL);
assert(info.nextExpireTime == 0); assert(info.nextExpireTime == EB_EXPIRE_TIME_INVALID);
} else { } else {
assert(info.nextExpireTime == expireTime); assert(info.nextExpireTime == expireTime);
} }
...@@ -2372,6 +2388,7 @@ int ebucketsTest(int argc, char **argv, int flags) { ...@@ -2372,6 +2388,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
.itemsExpired = 0}; .itemsExpired = 0};
ebExpire(&eb, &myEbucketsType2, &info); ebExpire(&eb, &myEbucketsType2, &info);
assert(info.itemsExpired == (uint64_t) numItems); assert(info.itemsExpired == (uint64_t) numItems);
assert(info.nextExpireTime == (uint64_t)updateItemTo << EB_BUCKET_KEY_PRECISION);
assert(ebGetTotalItems(eb, &myEbucketsType2) == 1); assert(ebGetTotalItems(eb, &myEbucketsType2) == 1);
/* active-expire. Expected that all will be expired */ /* active-expire. Expected that all will be expired */
...@@ -2383,6 +2400,7 @@ int ebucketsTest(int argc, char **argv, int flags) { ...@@ -2383,6 +2400,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
.itemsExpired = 0}; .itemsExpired = 0};
ebExpire(&eb, &myEbucketsType2, &info2); ebExpire(&eb, &myEbucketsType2, &info2);
assert(info2.itemsExpired == (uint64_t) 1); assert(info2.itemsExpired == (uint64_t) 1);
assert(info2.nextExpireTime == EB_EXPIRE_TIME_INVALID);
assert(ebGetTotalItems(eb, &myEbucketsType2) == 0); assert(ebGetTotalItems(eb, &myEbucketsType2) == 0);
ebDestroy(&eb, &myEbucketsType2, NULL); ebDestroy(&eb, &myEbucketsType2, NULL);
......
...@@ -250,11 +250,9 @@ typedef struct ExpireInfo { ...@@ -250,11 +250,9 @@ typedef struct ExpireInfo {
uint64_t maxToExpire; /* [INPUT ] Limit of number expired items to scan */ uint64_t maxToExpire; /* [INPUT ] Limit of number expired items to scan */
void *ctx; /* [INPUT ] context to pass to onExpireItem */ void *ctx; /* [INPUT ] context to pass to onExpireItem */
uint64_t now; /* [INPUT ] Current time in msec. */ uint64_t now; /* [INPUT ] Current time in msec. */
uint64_t nextExpireTime; /* [OUTPUT] Next expiration time. Return 0, if none left. */
/* TODO: Distinct between expired & updated */
uint64_t itemsExpired; /* [OUTPUT] Returns the number of expired or updated items. */ uint64_t itemsExpired; /* [OUTPUT] Returns the number of expired or updated items. */
uint64_t nextExpireTime; /* [OUTPUT] Next expiration time. Returns
EB_EXPIRE_TIME_INVALID if none left. */
} ExpireInfo; } ExpireInfo;
/* ebuckets API */ /* ebuckets API */
......
...@@ -446,7 +446,7 @@ void listpackExExpire(redisDb *db, robj *o, ExpireInfo *info) { ...@@ -446,7 +446,7 @@ void listpackExExpire(redisDb *db, robj *o, ExpireInfo *info) {
lpt->lp = lpDeleteRange(lpt->lp, 0, expired * 3); lpt->lp = lpDeleteRange(lpt->lp, 0, expired * 3);
min = hashTypeGetNextTimeToExpire(o); min = hashTypeGetNextTimeToExpire(o);
info->nextExpireTime = (min != EB_EXPIRE_TIME_INVALID) ? min : 0; info->nextExpireTime = min;
} }
static void listpackExAddInternal(robj *o, listpackEntry ent[3]) { static void listpackExAddInternal(robj *o, listpackEntry ent[3]) {
...@@ -1885,7 +1885,7 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) { ...@@ -1885,7 +1885,7 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
activeExpireCtx->fieldsToExpireQuota -= info.itemsExpired; activeExpireCtx->fieldsToExpireQuota -= info.itemsExpired;
/* If hash has no more fields to expire, remove it from HFE DB */ /* If hash has no more fields to expire, remove it from HFE DB */
if (info.nextExpireTime == 0) { if (info.nextExpireTime == EB_EXPIRE_TIME_INVALID) {
if (hashTypeLength(hashObj, 0) == 0) { if (hashTypeLength(hashObj, 0) == 0) {
robj *key = createStringObject(keystr, sdslen(keystr)); robj *key = createStringObject(keystr, sdslen(keystr));
dbDelete(activeExpireCtx->db, key); dbDelete(activeExpireCtx->db, key);
...@@ -1896,10 +1896,9 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) { ...@@ -1896,10 +1896,9 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
} }
return ACT_REMOVE_EXP_ITEM; return ACT_REMOVE_EXP_ITEM;
} else { } else {
/* Hash has more fields to expire. Keep hash to pending items that will /* Hash has more fields to expire. Update next expiration time of the hash
* be added back to global HFE DS at the end of ebExpire() */ * and indicate to add it back to global HFE DS */
ExpireMeta *expireMeta = hashGetExpireMeta(hashObj); ebSetMetaExpTime(hashGetExpireMeta(hashObj), info.nextExpireTime);
ebSetMetaExpTime(expireMeta, info.nextExpireTime);
return ACT_UPDATE_EXP_ITEM; return ACT_UPDATE_EXP_ITEM;
} }
} }
......
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