Unverified Commit 2bce71b5 authored by Harkrishn Patro's avatar Harkrishn Patro Committed by GitHub
Browse files

Exit early if slowlog/acllog max len set to zero (#12965)

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.
parent e12f2dec
...@@ -2661,6 +2661,15 @@ void ACLUpdateInfoMetrics(int reason){ ...@@ -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 /* 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 * 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 * to find similar entries in the current log in order to bump the counter of
...@@ -2680,6 +2689,11 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username ...@@ -2680,6 +2689,11 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
/* Update ACL info metrics */ /* Update ACL info metrics */
ACLUpdateInfoMetrics(reason); ACLUpdateInfoMetrics(reason);
if (server.acllog_max_len == 0) {
trimACLLogEntriesToMaxLen();
return;
}
/* Create a new entry. */ /* Create a new entry. */
struct ACLLogEntry *le = zmalloc(sizeof(*le)); struct ACLLogEntry *le = zmalloc(sizeof(*le));
le->count = 1; le->count = 1;
...@@ -2742,12 +2756,7 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username ...@@ -2742,12 +2756,7 @@ void addACLLogEntry(client *c, int reason, int context, int argpos, sds username
* to its maximum size. */ * to its maximum size. */
ACLLogEntryCount++; /* Incrementing the entry_id count to make each record in the log unique. */ ACLLogEntryCount++; /* Incrementing the entry_id count to make each record in the log unique. */
listAddNodeHead(ACLLog, le); listAddNodeHead(ACLLog, le);
while(listLength(ACLLog) > server.acllog_max_len) { trimACLLogEntriesToMaxLen();
listNode *ln = listLast(ACLLog);
ACLLogEntry *le = listNodeValue(ln);
ACLFreeLogEntry(le);
listDelNode(ACLLog,ln);
}
} }
} }
......
...@@ -279,7 +279,7 @@ sds createLatencyReport(void) { ...@@ -279,7 +279,7 @@ sds createLatencyReport(void) {
/* Potentially commands. */ /* Potentially commands. */
if (!strcasecmp(event,"command")) { 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; advise_slowlog_enabled = 1;
advices++; advices++;
} else if (server.slowlog_log_slower_than/1000 > } else if (server.slowlog_log_slower_than/1000 >
......
...@@ -121,7 +121,7 @@ void slowlogInit(void) { ...@@ -121,7 +121,7 @@ void slowlogInit(void) {
* This function will make sure to trim the slow log accordingly to the * This function will make sure to trim the slow log accordingly to the
* configured max length. */ * configured max length. */
void slowlogPushEntryIfNeeded(client *c, robj **argv, int argc, long long duration) { 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) if (duration >= server.slowlog_log_slower_than)
listAddNodeHead(server.slowlog, listAddNodeHead(server.slowlog,
slowlogCreateEntry(c,argv,argc,duration)); slowlogCreateEntry(c,argv,argc,duration));
......
...@@ -802,6 +802,16 @@ start_server {tags {"acl external:skip"}} { ...@@ -802,6 +802,16 @@ start_server {tags {"acl external:skip"}} {
assert {[dict get $entry username] eq {antirez}} 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} { test {ACL LOG entries are limited to a maximum amount} {
r ACL LOG RESET r ACL LOG RESET
r CONFIG SET acllog-max-len 5 r CONFIG SET acllog-max-len 5
...@@ -813,6 +823,11 @@ start_server {tags {"acl external:skip"}} { ...@@ -813,6 +823,11 @@ start_server {tags {"acl external:skip"}} {
assert {[llength [r ACL LOG]] == 5} 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} { test {When default user is off, new connections are not authenticated} {
r ACL setuser default off r ACL setuser default off
catch {set rd1 [redis_deferring_client]} e catch {set rd1 [redis_deferring_client]} e
......
...@@ -14,6 +14,16 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} { ...@@ -14,6 +14,16 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
assert_equal [r slowlog len] 1 assert_equal [r slowlog len] 1
} {} {needs:debug} } {} {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} { test {SLOWLOG - max entries is correctly handled} {
r config set slowlog-log-slower-than 0 r config set slowlog-log-slower-than 0
r config set slowlog-max-len 10 r config set slowlog-max-len 10
...@@ -42,7 +52,7 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} { ...@@ -42,7 +52,7 @@ start_server {tags {"slowlog"} overrides {slowlog-log-slower-than 1000000}} {
set e [lindex [r slowlog get] 0] set e [lindex [r slowlog get] 0]
assert_equal [llength $e] 6 assert_equal [llength $e] 6
if {!$::external} { if {!$::external} {
assert_equal [lindex $e 0] 107 assert_equal [lindex $e 0] 106
} }
assert_equal [expr {[lindex $e 2] > 100000}] 1 assert_equal [expr {[lindex $e 2] > 100000}] 1
assert_equal [lindex $e 3] {debug sleep 0.2} assert_equal [lindex $e 3] {debug sleep 0.2}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment