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

Add batch-lines support to Redis destination #3745

Merged
merged 4 commits into from Aug 18, 2021

Conversation

OverOrion
Copy link
Collaborator

@OverOrion OverOrion commented Aug 2, 2021

This PR adds batch-lines() support for the redis destination driver which increases performance.
Performance improvement is significant (8665U, 32 GiB RAM): [1]

8 workers, no batch-lines() option 8 workers, batch-lines(100)
~ 80k/s ~ 240k/s (peak 450k/s)

The batching is based on the pipelining feature of Redis. We have to count the sent (appended) messages as we must retrieve as many as we sent.

I also tested that when the Redis server goes offline it will reconnect.

[1] I used the following configuration:

source s_local {
        network(
        port(4444)
        max-connections(100)
        log-iw-size(100000)
        log-fetch-limit(1000)
        );
};
destination d_redis {
    redis(
        host("localhost")
        port(6379)
        command("HINCRBY", "hosts", "$HOST", "1")
        workers(8)
        log-fifo-size(100000)
        batch-lines(100)
    );
};

log {
        source(s_local);
        destination(d_redis);
        flags(flow-control);
};

And sent messages via loggen:
./loggen --permanent --rate=10000000 --active-connections 10 -i localhost 4444

Signed-off-by: Parrag Szilárd szilard.parrag@gmail.com

@kira-syslogng
Copy link
Contributor

Build SUCCESS

Barkodcz
Barkodcz previously approved these changes Aug 2, 2021
Copy link
Contributor

@Barkodcz Barkodcz left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@Kokan Kokan self-requested a review August 2, 2021 19:45
MrAnno
MrAnno previously requested changes Aug 2, 2021
Copy link
Collaborator

@MrAnno MrAnno left a comment

Choose a reason for hiding this comment

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

Please add a news file entry about the new options: batch-lines() and batch-timeout().

modules/redis/redis-worker.c Outdated Show resolved Hide resolved
@kira-syslogng
Copy link
Contributor

Build SUCCESS

@kira-syslogng
Copy link
Contributor

Build SUCCESS

modules/redis/redis-worker.c Outdated Show resolved Hide resolved
modules/redis/redis-worker.h Outdated Show resolved Hide resolved
modules/redis/redis-worker.c Outdated Show resolved Hide resolved
modules/redis/redis-worker.c Outdated Show resolved Hide resolved
news/feature-3745.md Show resolved Hide resolved
@MrAnno MrAnno requested review from MrAnno and removed request for MrAnno August 3, 2021 17:10
@MrAnno MrAnno dismissed their stale review August 3, 2021 17:11

Thank you

@kira-syslogng
Copy link
Contributor

Build FAILURE

@OverOrion OverOrion force-pushed the redis_pipeline_pr branch 2 times, most recently from 3e8c1bb to dcf5ceb Compare August 4, 2021 14:16
@kira-syslogng
Copy link
Contributor

Build SUCCESS

Signed-off-by: Parrag Szilárd <szilard.parrag@gmail.com>
If a worker had sent a message with pipelining, then during a normal
reconnection the server would not know how to answer the `getReply`
function as the bind address might have changed. [`redisReconnect`](https://github.com/redis/hiredis/blob/master/hiredis.h#L297) reuses
the same parameters as the initial connection.

Signed-off-by: Parrag Szilárd <szilard.parrag@gmail.com>
Signed-off-by: Parrag Szilárd <szilard.parrag@gmail.com>
Signed-off-by: Parrag Szilárd <szilard.parrag@gmail.com>
@kira-syslogng
Copy link
Contributor

Build SUCCESS

@Kokan Kokan added this to the syslog-ng-3.34 milestone Aug 18, 2021
@Kokan Kokan added the user-visible-feature User visible feature label Aug 18, 2021
@MrAnno MrAnno merged commit 9e76817 into syslog-ng:master Aug 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user-visible-feature User visible feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants