The problematic modification

Oct 13, 2025 by thesweetdesigns.com 101 views

Describe the bug

In a recent attempt to improve the performance of the SCAN command in Redis, a modification was made to move the TYPE filtering logic into the scan callback. This change was intended to avoid unnecessary lookupKey operations, which were previously performed during the main scan logic. However, this approach may have an unintended consequence on the performance of the ZSCAN command.

Understanding the issue

The ZSCAN command, like the SCAN command, uses the scanGenericCommand and scanCallback functions. However, the ZSCAN command will not enter the then statement in the scanGenericCommand function because the o variable is not null. As a result, the type checking within the then statement is not performed. This is in contrast to the SCAN command, which does perform type checking.

The problematic modification

The modification that was made to improve the performance of the SCAN command added an extra if check to the scan callback. This if check is not necessary for the ZSCAN command, as it will not enter the then statement in the scanGenericCommand function. However, due to the fact that the scan callback will be called many times, this modification introduces unnecessary overhead to the ZSCAN command.

To reproduce the issue

To reproduce the issue, you can use the following ZSCAN command: ZSCAN test_zadd 0 MATCH sortset*1* COUNT 100. You can test the execution time of the scanGenericCommand function by instrumentation. Before the modification, the average execution time was 73us. After the modification, the average execution time is ~78us, which is a 5% regression.

Expected behavior

The expected behavior is that the performance improvement attempt for the SCAN command does not cause a performance issue for the ZSCAN command. However, the modification made to improve the performance of the SCAN command has an unintended consequence on the performance of the ZSCAN command.

Impact on performance

The modification made to improve the performance of the SCAN command introduces unnecessary overhead to the ZSCAN command. This overhead is due to the extra if check in the scan callback, which is not necessary for the ZSCAN command. As a result, the ZSCAN command experiences a 5% regression in performance.

Conclusion

In conclusion, the performance improvement attempt for the SCAN command makes the ZSCAN command slower. The modification made to improve the performance of the SCAN command introduces unnecessary overhead to the ZSCAN command, resulting in a 5% regression in performance. This issue highlights the importance of thoroughly testing and reviewing changes to ensure that they do not have unintended consequences on other parts of the system.

Recommendations

Based on the analysis of the issue, the following recommendations are made:

The modification made to improve the performance of the SCAN command should be reverted to its original state.
The scan callback should be modified to remove the extra if check, which is not necessary for the ZSCAN command.
Thorough testing and review should be performed to ensure that changes do not have unintended consequences on other parts of the system.

Future work

Future work should focus on improving the performance of the ZSCAN command without introducing unnecessary overhead. This can be achieved by optimizing the scan callback and removing unnecessary if checks. Additionally, thorough testing and review should be performed to ensure that changes do not have unintended consequences on other parts of the system.

Code analysis

The code analysis of the scanGenericCommand function and the scanCallback function reveals the following:

The scanGenericCommand function has an if check to filter expired keys. This if check is not necessary for the ZSCAN command, as it will not enter the then statement.
The scanCallback function has an extra if check to filter elements based on their type. This if check is not necessary for the ZSCAN command, as it will not enter the then statement.

Commit history

The commit history of the modification made to improve the performance of the SCAN command is as follows:

Commit #7f0a7f0: "Improve performance of SCAN command by moving TYPE filtering logic into scan callback."

Conclusion

Code snippets

scanGenericCommand function

void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
    ...
    /* Step 3: Filter the expired keys */
    if (o == NULL && listLength(keys)) {
        robj kobj;
        listIter li;
        listNode *ln;
        listRewind(keys, &li);
        while ((ln = listNext(&li))) {
            sds key = listNodeValue(ln);
            initStaticStringObject(kobj, key);
            /* Filter an element if it isn't the type we want. */
            /* TODO: remove this in redis 8.0 */
            if (typename) {
                robj* typecheck = lookupKeyReadWithFlags(c->db, &kobj, LOOKUP_NOTOUCH|LOOKUP_NONOTIFY);
                if (!typecheck || !objectTypeCompare(typecheck, type)) {
                    listDelNode(keys, ln);
                }
                continue;
            }
            if (expireIfNeeded(c->db, &kobj, 0)) {
                listDelNode(keys, ln);
            }
        }
    }
    ...
}

scanCallback function

void scanCallback(void *privdata, const dictEntry *de) {
    ...
    /* Filter an element if it isn't the type we want. */
    if (!o && data->type != LLONG_MAX) {
        robj *rval = dictGetVal(de);
        if (!objectTypeCompare(rval, data->type)) return;
    }
    ...
}

Commit history

Commit #7f0a7f0: "Improve performance of SCAN command by moving TYPE filtering logic into scan callback."
Q&A: The performance improvement attempt for the SCAN command makes the ZSCAN command slower ====================================================================================

Q: What is the issue with the performance improvement attempt for the SCAN command?

A: The performance improvement attempt for the SCAN command makes the ZSCAN command slower. The modification made to improve the performance of the SCAN command introduces unnecessary overhead to the ZSCAN command, resulting in a 5% regression in performance.

Q: What is the cause of the performance issue?

A: The cause of the performance issue is the extra if check in the scan callback, which is not necessary for the ZSCAN command. This if check is introduced by the modification made to improve the performance of the SCAN command.

Q: How does the ZSCAN command use the scanGenericCommand and scanCallback functions?

A: The ZSCAN command uses the scanGenericCommand and scanCallback functions, but it will not enter the then statement in the scanGenericCommand function because the o variable is not null. As a result, the type checking within the then statement is not performed.

Q: What is the impact of the performance issue on the ZSCAN command?

A: The performance issue has a significant impact on the ZSCAN command, resulting in a 5% regression in performance. This means that the ZSCAN command takes longer to execute than it did before the modification.

Q: How can the performance issue be fixed?

A: The performance issue can be fixed by reverting the modification made to improve the performance of the SCAN command. This will remove the extra if check in the scan callback, which is not necessary for the ZSCAN command.

Q: What are the implications of the performance issue?

A: The performance issue has significant implications for the ZSCAN command, as it can impact the performance of the Redis database as a whole. This is because the ZSCAN command is used to scan the sorted set data structure, which is a critical component of the Redis database.

Q: How can the performance issue be prevented in the future?

A: The performance issue can be prevented in the future by thoroughly testing and reviewing changes to ensure that they do not have unintended consequences on other parts of the system. This includes testing the ZSCAN command and other related commands to ensure that they are not impacted by changes to the SCAN command.

Q: What is the recommended course of action to fix the performance issue?

A: The recommended course of action to fix the performance issue is to revert the modification made to improve the performance of the SCAN command. This will remove the extra if check in the scan callback, which is not necessary for the ZSCAN command.

Q: What are the benefits of fixing the performance issue?

A: The benefits of fixing the performance issue include improved performance of the ZSCAN command, reduced latency, and improved overall performance of the Redis database.

Q: How can the performance issue be monitored and tracked?

A: The performance issue can be monitored and tracked by using Redis performance monitoring tools, such as Redis Insight or Redis Studio. These tools can provide detailed information about the performance of the ZSCAN command and other related commands.

Q: What are the best practices for preventing performance issues in the future?

A: The best practices for preventing performance issues in the future include:

Thoroughly testing and reviewing changes to ensure that they do not have unintended consequences on other parts of the system.
Testing the ZSCAN command and other related commands to ensure that they are not impacted by changes to the SCAN command.
Using Redis performance monitoring tools to monitor and track performance issues.
Regularly reviewing and updating the Redis database to ensure that it is optimized for performance.

Code snippets

scanGenericCommand function

void scanGenericCommand(client *c, robj *o, unsigned long long cursor) {
    ...
    /* Step 3: Filter the expired keys */
    if (o == NULL && listLength(keys)) {
        robj kobj;
        listIter li;
        listNode *ln;
        listRewind(keys, &li);
        while ((ln = listNext(&li))) {
            sds key = listNodeValue(ln);
            initStaticStringObject(kobj, key);
            /* Filter an element if it isn't the type we want. */
            /* TODO: remove this in redis 8.0 */
            if (typename) {
                robj* typecheck = lookupKeyReadWithFlags(c->db, &kobj, LOOKUP_NOTOUCH|LOOKUP_NONOTIFY);
                if (!typecheck || !objectTypeCompare(typecheck, type)) {
                    listDelNode(keys, ln);
                }
                continue;
            }
            if (expireIfNeeded(c->db, &kobj, 0)) {
                listDelNode(keys, ln);
            }
        }
    }
    ...
}

scanCallback function

void scanCallback(void *privdata, const dictEntry *de) {
    ...
    /* Filter an element if it isn't the type we want. */
    if (!o && data->type != LLONG_MAX) {
        robj *rval = dictGetVal(de);
        if (!objectTypeCompare(rval, data->type)) return;
    }
    ...
}

Commit history

Commit #7f0a7f0: "Improve performance of SCAN command by moving TYPE filtering logic into scan callback."