Skip to content

fix(log-rotate): reopen logs after partial rotation#13375

Draft
juzhiyuan wants to merge 2 commits into
masterfrom
fix/log-rotate-partial-reopen
Draft

fix(log-rotate): reopen logs after partial rotation#13375
juzhiyuan wants to merge 2 commits into
masterfrom
fix/log-rotate-partial-reopen

Conversation

@juzhiyuan
Copy link
Copy Markdown
Member

Description

Fixes #13368.

This updates log-rotate so a partial rotation failure does not prevent Nginx from reopening the log files that were rotated successfully. The regression test covers the reported behavior where a custom plugin writes through core.log.info after rotation and access logs should continue landing in the current access.log.

Root cause

rotate_file() iterates over the configured log files in apisix/plugins/log-rotate.lua. Before this patch, if rename_file() returned nil for any configured file, rotate_file() returned immediately from the loop before reaching the USR1 reopen step.

That means this sequence could happen:

  1. error.log is renamed to a timestamped file.
  2. access.log is missing, so rename_file() returns nil.
  3. rotate_file() returns before sending USR1 to the master process.
  4. The master keeps writing through stale file descriptors, so later plugin logs and access logs do not appear at the canonical current log paths.

Fix

  • Continue rotating the remaining configured files when one file is missing or cannot be renamed.
  • Track only files that actually have a rotated target.
  • Send USR1 if at least one log file was rotated.
  • Keep the degenerate both-files-missing case as a no-op by skipping USR1 when there are no rotated files.

Tests

  • Added a regression test in t/plugin/log-rotate.t using three pipelined requests:
    • /setup: configure a log-phase serverless-post-function, remove access.log, wait for partial rotation, then disable log-rotate to stabilize assertions.
    • /hello: trigger both plugin error-log output and access-log output.
    • /verify: assert the current error.log contains the plugin marker and current access.log contains the request.
  • The test explicitly avoids a single-request assertion shape because log-phase plugins run after the response has been sent.

Validation run:

PATH=/usr/local/openresty/bin:/usr/local/openresty/nginx/sbin:/usr/local/openresty/luajit/bin:$PATH \
FLUSH_ETCD=1 prove -Itest-nginx/lib -I. t/plugin/log-rotate.t

Result after the fix:

t/plugin/log-rotate.t .. ok
All tests successful.
Files=1, Tests=23, Result: PASS

Additional manual check:

  • Temporarily changed the regression setup to remove error.log while leaving access.log present; the same command passed all 23 tests after the fix.

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.

bug: The logrotate plugin causes custom plugin logs not to be printed

1 participant