Add config option to set max debug log size #24950

1. 
   tehelsper commented at 11:27 pm on April 22, 2022: none

   This PR adds a new config option “shrinkdebugfilesize=” which accepts an integer specifying the file size of the debug log in megabytes. This option can be specified alongside “shrinkdebugfile=1” (which is the default) and when shrinking the debug log, it will now shrink it to the size specified.

   The default size for debug.log shrinking is still 10 MB.

   I was working on a tool to analyze IBD times and my log kept shrinking if I wanted to change settings or needed to reboot for some reason. This was causing lost data and I didn’t want to let the log grow forever by setting shrinkdebugfile=0. That way I could leave it on passively and do post-analysis.

2. Add config option to set max debug log size 1471881501
3. Merge branch 'bitcoin:master' into master 8c5e5edc7f
4. 
   DrahtBot commented at 5:09 am on April 23, 2022: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #25614 (Severity-based logging, step 2 by jonatack)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

5. 
   laanwj commented at 8:31 am on April 28, 2022: member

   Thanks for your contribution. However, I’m not convinced that this doesn’t add unnecessary complexity. In the past we’ve rejected features that add more fully fledged log management to bitcoind because, outside trivial usage, it’s better to leave this to some dedicate external system such as logrotate.

   This adds an extra command line option that adds configuration complexity, and also needs tests.

   In your specific case I’d have recommended to disable the log shrinking completely for the duration of the experiment. Then you’re 100% sure to not lose data, where heuristically choosing a new number does not.

6. 
   tehelsper commented at 3:07 am on April 29, 2022: none

   I see your point of view and would definitely agree if it was functionality to rotate logs or manage them in some way. I do disagree on this specific change since it only allows a user to configure a value that is hard-coded internally.

   Since there is an internally hard-coded limit of 10M, it seems reasonable to allow a user to change it without any additional complexity. I would have found it more understandable if I saw this as a config option. It took some digging to even understand why my log seemed to be shrinking to a seemingly arbitrary number and having an argument in the help text indicating the default and the ability to change it may be helpful.

7. 
   tehelsper commented at 3:11 am on April 29, 2022: none

   I still need to get back and do some cleanup based on the linter and look more into automated testing. Manual testing for my test case has worked well.
8. 
   ghost commented at 4:25 am on April 29, 2022: none

   Since there is an internally hard-coded limit of 10M, it seems reasonable to allow a user to change it without any additional complexity.

   Makes sense. Concept ACK.

   This pull request is less complex as compared to #22350 and others

9. 
   luke-jr commented at 11:28 pm on May 9, 2022: member

   I would have expected the size to be used as the max debug log size, not the size to shrink to only when it’s 10% higher.

   Also, commit history is dirty (had a merge from master)

10. 
    laanwj commented at 9:08 am on May 10, 2022: member

    Since there is an internally hard-coded limit of 10M, it seems reasonable to allow a user to change it without any additional complexity.

    Sure, but it’s not entirely without maintenance burden. It means the code has to handle different sizes someone might configure. For example, the shrinking itself reads the entire size into memory, to clear the file and write it out again. This strategy works for a small size like 10MB, but if people set it to 1GB or more they may be in for a surprise.

11. 
    tehelsper commented at 5:10 am on May 19, 2022: none

    @laanwj Yeah, I agree a user could shoot themselves in the foot, but is that something that is typically prevented? Adding code to stop users from hurting themselves would add even more maintenance burden. Is there precedent for handing that when setting mempool size, dbcache, etc…?
12. DrahtBot added the label Utils/log/libs on Jul 1, 2022
13. DrahtBot added the label Needs rebase on Sep 1, 2022
14. 
    DrahtBot commented at 9:16 pm on September 1, 2022: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

    Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.

15. 
    achow101 commented at 7:13 pm on October 12, 2022: member

    Are you still working on this?
16. 
    tehelsper commented at 7:23 pm on October 12, 2022: none

    It sounded a bit contentious. I’ll go ahead and close it out.
17. tehelsper closed this on Oct 12, 2022

    ---

18. bitcoin locked this on Oct 12, 2023

Contributors
tehelsper DrahtBot laanwj ghost luke-jr achow101

Labels
Utils/log/libs Needs rebase