Skip to content

chore(logger): upgrade logforth to support global max_files limit for log_dir#17637

Merged
bohutang merged 3 commits intodatabendlabs:mainfrom
bohutang:dev-logforth-upgrade
Apr 9, 2025
Merged

chore(logger): upgrade logforth to support global max_files limit for log_dir#17637
bohutang merged 3 commits intodatabendlabs:mainfrom
bohutang:dev-logforth-upgrade

Conversation

@bohutang
Copy link
Member

@bohutang bohutang commented Mar 21, 2025

I hereby agree to the terms of the CLA available at: https://docs.databend.com/dev/policies/cla/

Summary

Upgrade logforth with Global File Limit Capability

Upgrades to logforth from https://github.com/datafuse-extras/logforth (global-max-files-v0.14 branch), which adds the ability to limit the total number of log files in log_dir via PR #1.

Tests

  • Unit Test
  • Logic Test
  • Benchmark Test
  • No Test - tests already in datafuse-extras/logforth

Type of change

  • Bug Fix (non-breaking change which fixes an issue)
  • New Feature (non-breaking change which adds functionality)
  • Breaking Change (fix or feature that could cause existing functionality not to work as expected)
  • Documentation Update
  • Refactoring
  • Performance Improvement
  • Other (please describe): enhance the log files limitition

This change is Reviewable

@github-actions github-actions bot added the pr-chore this PR only has small changes that no need to record, like coding styles. label Mar 21, 2025
@bohutang bohutang changed the title chore: upgrade logforth to support max_files limit chore(logger): upgrade logforth to support max_files limit Mar 21, 2025
@bohutang bohutang requested a review from Xuanwo March 21, 2025 09:41
@bohutang bohutang marked this pull request as ready for review March 21, 2025 09:41
@Xuanwo
Copy link
Member

Xuanwo commented Mar 21, 2025

This is a temporary solution, right? I see logforth 0.23 has already supported the max file limit: https://github.com/fast/logforth/blob/d9620447ba9be53a730d8960f103445de79076a9/src/append/rolling_file/rolling.rs#L137-L142

@bohutang
Copy link
Member Author

bohutang commented Mar 21, 2025

This is a temporary solution, right? I see logforth 0.23 has already supported the max file limit: https://github.com/fast/logforth/blob/d9620447ba9be53a730d8960f103445de79076a9/src/append/rolling_file/rolling.rs#L137-L142

No, this is the final solution.
The max_files is for rotation(if hourly the max_files mean not over max_files in a hour) not global all in log_dir, see : fast/logforth#107
BTW, it's hard to use the latest version of logforth, there are many api need to change, so we don't have the plan to upgrade to the latest version.

@bohutang bohutang changed the title chore(logger): upgrade logforth to support max_files limit chore(logger): upgrade logforth to support global max_files limit for log_dir Mar 21, 2025
@wubx
Copy link
Member

wubx commented Mar 23, 2025

The log section with:

[log.file]
limit = 12
max-size=1024
format = "text"
include_node_id=true
dir = "/var/log/databend"
level = "WARN,databend=info,opendal=info,openraft=info"

Test use

for i in `seq 1 1000` ; do echo "select $i"|bendsql ; done

"It feels like the number of generated log files is far beyond what was expected."

image

@bohutang bohutang marked this pull request as draft March 24, 2025 08:30
@bohutang
Copy link
Member Author

bohutang commented Apr 9, 2025

cc @wubx

From my local test, this works as expected.

[log.file]
limit = 3
max-size=256
format = "text"
include_node_id=true
dir = "./logs"
level = "INFO"

I only tested as:
Case 1:
The logs have many log files(64) and start the query, the log files reduce to 3.

Case 2:
Do many times like select sum(numbers) from numbers(10000), the logs directory always keep 3 log files.

@bohutang bohutang marked this pull request as ready for review April 9, 2025 05:45
@bohutang bohutang merged commit aae5b3a into databendlabs:main Apr 9, 2025
79 checks passed
@tisonkun
Copy link
Contributor

tisonkun commented Jan 3, 2026

From the latest code I don't see the date time being used as part of filename (prefix), then we may not need this workaround now?

See fast/logforth#207.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-chore this PR only has small changes that no need to record, like coding styles.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

chore: enable limit total number of log files

4 participants