Skip to content

Commit

Permalink
Exit early if slowlog/acllog max len set to zero (redis#12965)
Browse files Browse the repository at this point in the history
Currently slowlog gets disabled if slowlog-log-slower-than is set to less than zero. I think we should also disable it if slowlog-max-len is set to zero. We apply the same logic to acllog-max-len.
  • Loading branch information
hpatro authored Jan 23, 2024
1 parent e12f2de commit 2bce71b
Show file tree
Hide file tree
Showing 5 changed files with 43 additions and 9 deletions.
21 changes: 15 additions & 6 deletions src/acl.c
Original file line number Diff line number Diff line change
Expand Up @@ -2661,6 +2661,15 @@ void ACLUpdateInfoMetrics(int reason){
}
}

static void trimACLLogEntriesToMaxLen(void) {
while(listLength(ACLLog) > server.acllog_max_len) {
listNode *ln = listLast(ACLLog);
ACLLogEntry *le = listNodeValue(ln);
ACLFreeLogEntry(le);
listDelNode(ACLLog,ln);
}
}

/* Adds a new entry in the ACL log, making sure to delete the old entry
* if we reach the maximum length allowed for the log. This function attempts
* to find similar entries in the current log in order to bump the counter of
Expand All @@ -2680,6 +2689,11 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
/* Update ACL info metrics */
ACLUpdateInfoMetrics(reason);

if (server.acllog_max_len == 0) {
trimACLLogEntriesToMaxLen();
return;
}

/* Create a new entry. */
struct ACLLogEntry *le = zmalloc(sizeof(*le));
le->count = 1;
Expand Down Expand Up @@ -2742,12 +2756,7 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
* to its maximum size. */
ACLLogEntryCount++; /* Incrementing the entry_id count to make each record in the log unique. */
listAddNodeHead(ACLLog, le);
while(listLength(ACLLog) > server.acllog_max_len) {
listNode *ln = listLast(ACLLog);
ACLLogEntry *le = listNodeValue(ln);
ACLFreeLogEntry(le);
listDelNode(ACLLog,ln);
}
trimACLLogEntriesToMaxLen();
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/latency.c
Original file line number Diff line number Diff line change
Expand Up @@ -279,7 +279,7 @@ sds createLatencyReport(void) {

/* Potentially commands. */
if (!strcasecmp(event,"command")) {
if (server.slowlog_log_slower_than < 0) {
if (server.slowlog_log_slower_than < 0 || server.slowlog_max_len == 0) {
advise_slowlog_enabled = 1;
advices++;
} else if (server.slowlog_log_slower_than/1000 >
Expand Down
2 changes: 1 addition & 1 deletion src/slowlog.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ void slowlogInit(void) {
* This function will make sure to trim the slow log accordingly to the
* configured max length. */
void slowlogPushEntryIfNeeded(client *c, robj **argv, int argc, long long duration) {
if (server.slowlog_log_slower_than < 0) return; /* Slowlog disabled */
if (server.slowlog_log_slower_than < 0 || server.slowlog_max_len == 0) return; /* Slowlog disabled */
if (duration >= server.slowlog_log_slower_than)
listAddNodeHead(server.slowlog,
slowlogCreateEntry(c,argv,argc,duration));
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/acl.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -802,6 +802,16 @@ start_server {tags {"acl external:skip"}} {
assert {[dict get $entry username] eq {antirez}}
}

test {ACLLOG - zero max length is correctly handled} {
r ACL LOG RESET
r CONFIG SET acllog-max-len 0
for {set j 0} {$j < 10} {incr j} {
catch {r SET obj:$j 123}
}
r AUTH default ""
assert {[llength [r ACL LOG]] == 0}
}

test {ACL LOG entries are limited to a maximum amount} {
r ACL LOG RESET
r CONFIG SET acllog-max-len 5
Expand All @@ -813,6 +823,11 @@ start_server {tags {"acl external:skip"}} {
assert {[llength [r ACL LOG]] == 5}
}

test {ACL LOG entries are still present on update of max len config} {
r CONFIG SET acllog-max-len 0
assert {[llength [r ACL LOG]] == 5}
}

test {When default user is off, new connections are not authenticated} {
r ACL setuser default off
catch {set rd1 [redis_deferring_client]} e
Expand Down
12 changes: 11 additions & 1 deletion tests/unit/slowlog.tcl
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,16 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
assert_equal [r slowlog len] 1
} {} {needs:debug}

test {SLOWLOG - zero max length is correctly handled} {
r SLOWLOG reset
r config set slowlog-max-len 0
r config set slowlog-log-slower-than 0
for {set i 0} {$i < 100} {incr i} {
r ping
}
r slowlog len
} {0}

test {SLOWLOG - max entries is correctly handled} {
r config set slowlog-log-slower-than 0
r config set slowlog-max-len 10
Expand Down Expand Up @@ -42,7 +52,7 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
set e [lindex [r slowlog get] 0]
assert_equal [llength $e] 6
if {!$::external} {
assert_equal [lindex $e 0] 107
assert_equal [lindex $e 0] 106
}
assert_equal [expr {[lindex $e 2] > 100000}] 1
assert_equal [lindex $e 3] {debug sleep 0.2}
Expand Down

0 comments on commit 2bce71b

Please sign in to comment.