Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[database] Enable multi-threading feature in Redis #9284

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

xjasonlyu
Copy link
Contributor

Why I did it

Improve the performance of the redis-server in database container.

How I did it

Modify the redis.conf file to enable this feature.

How to verify it

The redis official configuration file redis.conf, and the config file in /etc/redis/.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106

Description for the changelog

This PR enables multi-threading feature that was supported by Redis 6.

A picture of a cute animal (not mandatory but encouraged)

@xjasonlyu xjasonlyu requested a review from lguohan as a code owner November 17, 2021 06:56
@akokhan
Copy link
Contributor

akokhan commented Nov 17, 2021

How does this improve the performance? Could you please provide some test results or the scenarios in which you see the improvement? Thanks.

@xjasonlyu
Copy link
Contributor Author

How does this improve the performance? Could you please provide some test results or the scenarios in which you see the improvement? Thanks.

Hi, after SONiC upgraded its Redis version from 5 to 6, the multi-thread options can be enabled to increase the read/write performance for each database.

Those articles have shown the benefits of enabling the multi-thread for Redis:

  1. https://developpaper.com/redis-6-0-multithreading-performance-test-results-and-analysis
  2. https://www.alibabacloud.com/blog/improving-redis-performance-through-multi-thread-processing_594150

image

@xjasonlyu
Copy link
Contributor Author

Here're some SONiC related tests & benchmarks:

With redis-server io-threads enabled, compared to the single-threaded redis-server, the more redis-dump clients that request the lower the time cost, and the same is true for the other clients (e.g. swssconfig)

redis-dump

Furthermore, with enabling multi-threading, we can reduce the complexity of multi-DB instances that currently used in SONiC, and by doing so, the time consumption when backing up and restoring the databases during warm-reboot would be significantly dropped.

Current warm-reboot SAVE/LOAD databases procedures (multi-DB instances):
Screen Shot 2021-12-01 at 6 03 54 PM

Simplified:
Screen Shot 2021-12-01 at 6 04 05 PM

@@ -1,5 +1,8 @@
#!/usr/bin/env bash

# Set io-threads to the number of physical CPU cores.
Copy link
Collaborator

@qiluo-msft qiluo-msft Dec 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

physical CPU cores

Seem you are assuming there is only one Redis instance on the host. However we have multi-ASIC use case that there are N Redis instances for N ASICs.

Then you set io-threads to N*cores. Will it harm the performance? #Closed

Copy link
Collaborator

@qiluo-msft qiluo-msft Dec 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the conf file document

# By default threading is disabled, we suggest enabling it only in machines
# that have at least 4 or more cores, leaving at least one spare core.
# Using more than 8 threads is unlikely to help much. We also recommend using
# threaded I/O only if you actually have performance problems, with Redis
# instances being able to use a quite big percentage of CPU time, otherwise
# there is no point in using this feature.
#
# So for instance if you have a four cores boxes, try to use 2 or 3 I/O
# threads, if you have a 8 cores, try to use 6 threads. In order to
# enable I/O threads use the following configuration directive:

Even for single Redis instance, it is recommended to leave spare core. And not to enable if < 4 cores.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After some testing, we've now decided to disable the multi-threading in multi-ASIC platforms and limit the thread numbers to between 2 to 8 threads.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the threads logic is like below:

# Retrieve total number of ASICs in the platform
num_asic = get_asic_nums()

# Retrieve total number of CPU cores in the platform
num_core = get_total_cores()

if num_asic > 1:
    # the multi-threading will be disabled in multi ASIC platform
    threads = 1
elif num_core < 4:
    # the multi-threading will be disabled if less than 4 cores
    threads = 1
else:
    threads = min(num_core//2, 8)

if threads > 1:
    enable_io_threads()

@hsluoyz
Copy link

hsluoyz commented Dec 6, 2021

Hi @dzhangalibaba can you provide some comments for this PR?

@ns-dzhang
Copy link

Hi @dzhangalibaba can you provide some comments for this PR?

Do you have any end-to-end tests on SONiC to make sure the performance improved? My understanding is the current CONFIG_DB -> APPL_DB -> ASIC_DB logic has the order limitation. Not sure if multi-thread really helps this case. Or multi-threading only helps the dump and save cases?

@xjasonlyu
Copy link
Contributor Author

Hi @ns-dzhang, cc @qiluo-msft

Was your previous multi-db performance test done in a physical environment or a virtual environment? Then we want to compare the performance of multi-threading and multi-DB instances. If it is convenient for you, can you provide some demo and data of your previous end-to-end test? Qi said that he has no data there.

@xjasonlyu
Copy link
Contributor Author

xjasonlyu commented Jan 20, 2022

Some tests have been made recently:

First, we analyzed the common DBs usage by capturing all SONiC database commands from its initial stage to running stage including the CONFIG_DB -> APPL_DB -> ASIC_DB logic flow, and this is the result we got.

DB

Also, we analyzed the GET/SET-liked command usage, and this is the approximate command type distribution.

CMD

Then we simulate the internal Redis requests by replaying the corresponding commands during its running stage. Several benchmarks have been made to compare the different performance when multithreading and multi-DB on/off.

The test environment details:

  • SONiC VS/VM with 4-core CPU
  • about 1.3 million commands tested
  • with 2 threads enabled for redis-server
  • with 5 threads for benchmark client to request

As for the multi-DB tests, two Redis instances were started and we assigned the ASIC_DB and APPL_DB to the second Redis instance. the database_config.json file is like below:

{
    "INSTANCES": {
        "redis": {
            "hostname": "127.0.0.1",
            "port": 6379,
            "unix_socket_path": "/var/run/redis/redis.sock"
        },
        "redis2": {
            "hostname": "127.0.0.1",
            "port": 6380,
            "unix_socket_path": "/var/run/redis/redis2.sock"
        }
    },
    "DATABASES": {
        "APPL_DB": {
            "id": 0,
            "separator": ":",
            "instance": "redis2"
        },
        "ASIC_DB": {
            "id": 1,
            "separator": ":",
            "instance": "redis2"
        },
        "COUNTERS_DB": {
            "id": 2,
            "separator": ":",
            "instance": "redis"
        },
        "LOGLEVEL_DB": {
            "id": 3,
            "separator": ":",
            "instance": "redis"
        },
...
}

The benchmark result:

BM

With multi-threading feature enabled, the performance is about 38% improved compare with single-threading version under this circumstance, and only 14% improved when multi-DB is enabled. Therefore, the result indicates that enabling the multi-threading feature can optimize the database container performance, and may even significantly improve it under heavy workload.

@qiluo-msft qiluo-msft requested a review from judyjoseph January 20, 2022 23:20
@xjasonlyu
Copy link
Contributor Author

xjasonlyu commented Feb 16, 2022

@ns-dzhang Hi, any update on this? #9284 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants