Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat-logger-update #1173

Merged
merged 9 commits into from
Sep 10, 2024
Merged

feat-logger-update #1173

merged 9 commits into from
Sep 10, 2024

Conversation

nguyenhoangthuan99
Copy link
Contributor

@nguyenhoangthuan99 nguyenhoangthuan99 commented Sep 9, 2024

Fixes #1153

@nguyenhoangthuan99 nguyenhoangthuan99 linked an issue Sep 9, 2024 that may be closed by this pull request
6 tasks
@nguyenhoangthuan99
Copy link
Contributor Author

nguyenhoangthuan99 commented Sep 9, 2024

  • separate log file for cortex.log and cortex-cli.log
  • pruning log file only to save last 100,000 lines. The log file will be truncated if number of line > max_lines. Check if file will be truncated every 1000 new lines.
  • configurable log file location. Save the config in the ~/.cortexrc
logFolderPath: C:\Users\jan\cortexcpp
dataFolderPath: C:\Users\jan\cortexcpp
host: 127.0.0.1
port: 3928

@dan-homebrew
Copy link
Contributor

dan-homebrew commented Sep 9, 2024

@nguyenhoangthuan99 Please link this PR to issue via keyword - I've helped to do this in the Description

@dan-homebrew
Copy link
Contributor

dan-homebrew commented Sep 9, 2024

@nguyenhoangthuan99 A few questions:

  • Do we have a filesize estimate of how much space 100,000 log lines will equate to?
  • Should we make num log lines a variable in .cortexrc that users can configure? (need to find a good variable name for this)

I imagine cortex.cpp may have some embedded use-cases in the future, where devs may have diskspace constraints for logs (difficult for me to imagine now with >1gb model sizes, but this may change in the future)

engine/utils/cortex_utils.h Outdated Show resolved Hide resolved
engine/utils/cortex_utils.h Outdated Show resolved Hide resolved
@nguyenhoangthuan99
Copy link
Contributor Author

nguyenhoangthuan99 commented Sep 9, 2024

Hi @dan-homebrew , Average each line will contains < 300 characters , each character is 1 byte. So 100,000 lines will be 300 * 100,000/(1024*1024) ~ 30Mb

@nguyenhoangthuan99 nguyenhoangthuan99 linked an issue Sep 9, 2024 that may be closed by this pull request
@dan-homebrew
Copy link
Contributor

Hi @dan-homebrew , Average each line will contains < 300 characters , each character is 1 byte. So 100,000 lines will be 300 * 100,000/(1024*1024) ~ 30Mb

This looks good to me - let's include it as a default param in .cortexrc

Copy link
Contributor

@dan-homebrew dan-homebrew left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@nguyenhoangthuan99 nguyenhoangthuan99 marked this pull request as ready for review September 10, 2024 02:48
@nguyenhoangthuan99 nguyenhoangthuan99 merged commit f25f6dd into dev Sep 10, 2024
4 checks passed
@nguyenhoangthuan99 nguyenhoangthuan99 deleted the feat-logger-update branch September 10, 2024 03:14
@dan-homebrew
Copy link
Contributor

Great job!

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.

epic: Cortex.cpp logs bug: Multiprocess cannot write log to the same file
4 participants