Skip to content

Commit

Permalink
Fix the logic of nextExpireTime at ebExpire()
Browse files Browse the repository at this point in the history
  • Loading branch information
@moticless
moticless committed Jun 2, 2024
1 parent 50569a9 commit b7e6ba3
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 16 deletions.
30 changes: 24 additions & 6 deletions src/ebuckets.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,8 @@ int ebSingleSegExpire(FirstSegHdr *firstSegHdr,
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) {
mIter->next = *updateList;
*updateList = iter;
Expand Down Expand Up @@ -457,6 +459,8 @@ static int ebSegExpire(FirstSegHdr *firstSegHdr,
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) {
mIter->next = *updateList;
*updateList = iter;
Expand Down Expand Up @@ -695,6 +699,8 @@ static int ebListExpire(ebuckets *eb,
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) {
metaItem->next = *updateList;
*updateList = item;
Expand All @@ -707,7 +713,7 @@ static int ebListExpire(ebuckets *eb,

if (expired == numItems) {
*eb = NULL;
info->nextExpireTime = 0;
info->nextExpireTime = EB_EXPIRE_TIME_INVALID;
return 1;
}

Expand Down Expand Up @@ -1458,11 +1464,14 @@ int ebAdd(ebuckets *eb, EbucketsType *type, eItem item, uint64_t expireTime) {
void ebExpire(ebuckets *eb, EbucketsType *type, ExpireInfo *info) {
/* updateList - maintain a list of expired items that the callback `onExpireItem`
* 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;

/* reset info outputs */
info->nextExpireTime = 0;
info->nextExpireTime = EB_EXPIRE_TIME_INVALID;
info->itemsExpired = 0;

/* if empty ebuckets */
Expand Down Expand Up @@ -1523,7 +1532,14 @@ void ebExpire(ebuckets *eb, EbucketsType *type, ExpireInfo *info) {
while (updateList) {
ExpireMeta *mItem = type->getExpireMeta(updateList);
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;
}

Expand Down Expand Up @@ -2179,7 +2195,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
assert(info.itemsExpired == 1);
if (i == numItems) { /* if last item */
assert(eb == NULL);
assert(info.nextExpireTime == 0);
assert(info.nextExpireTime == EB_EXPIRE_TIME_INVALID);
} else {
assert(info.nextExpireTime == EB_BUCKET_EXP_TIME(i));
}
Expand Down Expand Up @@ -2209,7 +2225,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
assert(info.itemsExpired == expirePerIter);
if (i == numIterations) { /* if last item */
assert(eb == NULL);
assert(info.nextExpireTime == 0);
assert(info.nextExpireTime == EB_EXPIRE_TIME_INVALID);
} else {
assert(info.nextExpireTime == expireTime);
}
Expand Down Expand Up @@ -2372,6 +2388,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
.itemsExpired = 0};
ebExpire(&eb, &myEbucketsType2, &info);
assert(info.itemsExpired == (uint64_t) numItems);
assert(info.nextExpireTime == (uint64_t)updateItemTo << EB_BUCKET_KEY_PRECISION);
assert(ebGetTotalItems(eb, &myEbucketsType2) == 1);

/* active-expire. Expected that all will be expired */
Expand All @@ -2383,6 +2400,7 @@ int ebucketsTest(int argc, char **argv, int flags) {
.itemsExpired = 0};
ebExpire(&eb, &myEbucketsType2, &info2);
assert(info2.itemsExpired == (uint64_t) 1);
assert(info2.nextExpireTime == EB_EXPIRE_TIME_INVALID);
assert(ebGetTotalItems(eb, &myEbucketsType2) == 0);

ebDestroy(&eb, &myEbucketsType2, NULL);
Expand Down
6 changes: 2 additions & 4 deletions src/ebuckets.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,9 @@ typedef struct ExpireInfo {
uint64_t maxToExpire; /* [INPUT ] Limit of number expired items to scan */
void *ctx; /* [INPUT ] context to pass to onExpireItem */
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 nextExpireTime; /* [OUTPUT] Next expiration time. Returns
EB_EXPIRE_TIME_INVALID if none left. */
} ExpireInfo;

/* ebuckets API */
Expand Down
11 changes: 5 additions & 6 deletions src/t_hash.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ void listpackExExpire(redisDb *db, robj *o, ExpireInfo *info) {
lpt->lp = lpDeleteRange(lpt->lp, 0, expired * 3);

min = hashTypeGetNextTimeToExpire(o);
info->nextExpireTime = (min != EB_EXPIRE_TIME_INVALID) ? min : 0;
info->nextExpireTime = min;
}

static void listpackExAddInternal(robj *o, listpackEntry ent[3]) {
Expand Down Expand Up @@ -1885,7 +1885,7 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
activeExpireCtx->fieldsToExpireQuota -= info.itemsExpired;

/* 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) {
robj *key = createStringObject(keystr, sdslen(keystr));
dbDelete(activeExpireCtx->db, key);
Expand All @@ -1896,10 +1896,9 @@ static ExpireAction hashTypeActiveExpire(eItem _hashObj, void *ctx) {
}
return ACT_REMOVE_EXP_ITEM;
} else {
/* Hash has more fields to expire. Keep hash to pending items that will
* be added back to global HFE DS at the end of ebExpire() */
ExpireMeta *expireMeta = hashGetExpireMeta(hashObj);
ebSetMetaExpTime(expireMeta, info.nextExpireTime);
/* Hash has more fields to expire. Update next expiration time of the hash
* and indicate to add it back to global HFE DS */
ebSetMetaExpTime(hashGetExpireMeta(hashObj), info.nextExpireTime);
return ACT_UPDATE_EXP_ITEM;
}
}
Expand Down

0 comments on commit b7e6ba3

Please sign in to comment.