bug: log-rotate max_kept doesn't work if enable_compression: true

[JohnsonCaii]

Description

$ git clone https://github.com/apache/apisix-docker.git
$ cd apisix-docker/example

Update the example/apisix_conf/config.yaml to:

plugins:
    - log-rotate

plugin_attr:
  log-rotate:
    interval: 1    # rotate interval (unit: second)
    max_kept: 10     # max number of log files will be kept
    max_size: -1      # max size of log files will be kept
    enable_compression: true

And then start the docker-compose.

docker-compose -p docker-apisix -f docker-compose-arm64.yml up -d

The number of files (*.tar.gz) in the example/apisix_log/ directory will grow without limit.

I want to keep only the X most recent tar.gz files while enable_compression: true, but the current configuration cannot take effect.

[tzssangglass]

Any error in the logs/error.log?

[JohnsonCaii]

Any error in the logs/error.log?

Seems like no.

$ cat 2022-11-17_01-45-11__error.log
2022/11/17 01:45:10 [notice] 115#115: signal process started
2022/11/17 01:45:10 [warn] 50#50: *4 [lua] plugin.lua:200: load(): new plugins: {"log-rotate":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 51#51: *1 [lua] plugin.lua:200: load(): new plugins: {"log-rotate":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 50#50: *4 [lua] plugin.lua:250: load_stream(): new plugins: {"syslog":true,"mqtt-proxy":true,"limit-conn":true,"ip-restriction":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 47#47: *2 [lua] plugin.lua:200: load(): new plugins: {"log-rotate":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 47#47: *2 [lua] plugin.lua:250: load_stream(): new plugins: {"syslog":true,"mqtt-proxy":true,"limit-conn":true,"ip-restriction":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 51#51: *1 [lua] plugin.lua:250: load_stream(): new plugins: {"syslog":true,"mqtt-proxy":true,"limit-conn":true,"ip-restriction":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 49#49: *5 [lua] plugin.lua:200: load(): new plugins: {"log-rotate":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 48#48: *3 [lua] plugin.lua:200: load(): new plugins: {"log-rotate":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 49#49: *5 [lua] plugin.lua:250: load_stream(): new plugins: {"syslog":true,"mqtt-proxy":true,"limit-conn":true,"ip-restriction":true}, context: init_worker_by_lua*
2022/11/17 01:45:10 [warn] 48#48: *3 [lua] plugin.lua:250: load_stream(): new plugins: {"syslog":true,"mqtt-proxy":true,"limit-conn":true,"ip-restriction":true}, context: init_worker_by_lua*
2022/11/17 01:45:11 [warn] 51#51: *161 [lua] log-rotate.lua:238: rotate_file(): send USR1 signal to master process [1] for reopening log file, context: ngx.timer

[JohnsonCaii]

https://github.com/apache/apisix/blob/master/apisix/plugins/log-rotate.lua#L179
The .tar.gz suffix is hardcoded here.

The code itself didn't check if compression was enabled when cleaning up old files here.
https://github.com/apache/apisix/blob/master/apisix/plugins/log-rotate.lua#L254

In the very first code of this feature, it's correct.
05fc230#diff-cec78832f4c2b34965a93f89b98b12104e34868088ae2a1380973a94d43c0a8aR125

Please have a check on it. Thanks

[monkeyDluffy6017]

The logic is correct:

1. rename the old log file
2. open the new log file
3. compress the old log file
4. clean the oldest file

[monkeyDluffy6017]

I'll try to reproduce your problem

[JohnsonCaii]

The logic is correct only if enable_compression: false. Please see my input here.

https://github.com/apache/apisix/blob/master/apisix/plugins/log-rotate.lua#L179 The .tar.gz suffix is hardcoded here.

The code itself didn't check if compression was enabled when cleaning up old files here. https://github.com/apache/apisix/blob/master/apisix/plugins/log-rotate.lua#L254

In the very first code of this feature, it's correct. 05fc230#diff-cec78832f4c2b34965a93f89b98b12104e34868088ae2a1380973a94d43c0a8aR125

Please have a check on it. Thanks

[monkeyDluffy6017]

The .tar.gz suffix is hardcoded here.

I don't know what you mean '.tar.gz' suffix is hardcoded, the file compressed is renamed before (https://github.com/apache/apisix/blob/master/apisix/plugins/log-rotate.lua#L227), and it just add a suffix (.tar.gz) here, it's full name likes: 2022-11-18_13-49-06__access.log.tar.gz
We have test case to check this: https://github.com/apache/apisix/blob/master/t/plugin/log-rotate2.t

[JohnsonCaii]

几个命令很容易复现，你试下吧。另外，测试用例里没有覆盖到 max-kept 跟 enable_compression: true 的场景

[guitu168]

I reproduced this problem~ I set to keep only 10 log files, but it didn't take effect.

cc @monkeyDluffy6017

[monkeyDluffy6017]

OK, the log-rotate plugin can not clean the compressed log file now, would you like to fix this? @JohnsonCaii

[JohnsonCaii]

Sorry. I'm not familiar with Lua, maybe someone here can help to fix this issue?

[JohnsonCaii]

And it would be great if log-rotate plugin have the gzip compress support since the .gz file can be viewed with the native commands like grep and less.