Increase minimum debug.log size to 10MB after shrink. #9625

morcos commented at 5:49 PM on January 24, 2017: member

-shrinkdebugfile defaults to true if no -debug options are passed, but 200k is an insufficient history to help users who may have restarted their nodes debug any issues.

MarcoFalke commented at 5:55 PM on January 24, 2017: member

utACK 338ad5e0a53bc153454bf1f3dc46de0f705419d2

On Tue, Jan 24, 2017 at 6:50 PM, Alex Morcos notifications@github.com wrote:

-shrinkdebugfile defaults to true if no -debug options are passed, but 200k is an insufficient history to help users who may have restarted their nodes debug any issues.

You can view, comment on, or merge this pull request online at:

#9625

Commit Summary

Increase minimum debug.log size to 9MB after shrink.

File Changes

M src/util.cpp (3)

Patch Links:

#9625.patch #9625.diff

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

paveljanik commented at 6:04 PM on January 24, 2017: contributor

utACK https://github.com/bitcoin/bitcoin/pull/9625/commits/338ad5e0a53bc153454bf1f3dc46de0f705419d2

btcdrak commented at 6:42 PM on January 24, 2017: contributor

utACK 338ad5e

gmaxwell commented at 6:52 PM on January 24, 2017: contributor

Yea, sure, though this also won't be enough in many cases, I think. utACK

sipa commented at 7:23 PM on January 24, 2017: member

Concept ACK, but can you introduce a named constant for this?

jonasschnelli added the label Docs and Output on Jan 24, 2017

jonasschnelli commented at 7:24 PM on January 24, 2017: contributor

utACK 338ad5e0a53bc153454bf1f3dc46de0f705419d2, agree about the const.

MarcoFalke commented at 7:51 PM on January 24, 2017: member

We might even extend the command line option to specify how many mb the user wants to store, but better leave the nits for past-0.15 branch-off.

TheBlueMatt commented at 10:37 PM on January 24, 2017: member

Looks good. Would also be nice to add a comment above where this is called in init noting - "Do this first since it both loads a bunch of debug.log into memory, and because this needs to happen before any other debug.log printing".

morcos force-pushed on Jan 25, 2017

Increase minimum debug.log size to 10MB after shrink. 29fb311858

morcos force-pushed on Jan 25, 2017

morcos renamed this:
~~Increase minimum debug.log size to 9MB after shrink.~~
Increase minimum debug.log size to 10MB after shrink.
on Jan 25, 2017

morcos commented at 1:38 AM on January 25, 2017: member

I think I addressed the feedback.

Incidentally I was looking into file_size and fseek to see whether I needed to have the 10% buffer, and it didn't look to me from the internets that the negative fseek from SEEK_END was necessarily a supported operation?

in src/util.cpp:None in 29fb311858

 729 |      boost::filesystem::path pathLog = GetDataDir() / "debug.log";
 730 |      FILE* file = fopen(pathLog.string().c_str(), "r");
 731 | -    if (file && boost::filesystem::file_size(pathLog) > 10 * 1000000)
 732 | +    // If debug.log file is more than 10% bigger the RECENT_DEBUG_HISTORY_SIZE
 733 | +    // trim it down by saving only the last RECENT_DEBUG_HISTORY_SIZE bytes
 734 | +    if (file && boost::filesystem::file_size(pathLog) > 11 * (RECENT_DEBUG_HISTORY_SIZE / 10))

MarcoFalke commented at 1:49 AM on January 25, 2017:

RECENT_DEBUG_HISTORY_SIZE * 11 / 10 might give better results for the integer division, when replaced by a command line option. Not that it matters, but µnit...

in src/util.cpp:None in 29fb311858

 727 | +    constexpr size_t RECENT_DEBUG_HISTORY_SIZE = 10 * 1000000;
 728 |      // Scroll debug.log if it's getting too big
 729 |      boost::filesystem::path pathLog = GetDataDir() / "debug.log";
 730 |      FILE* file = fopen(pathLog.string().c_str(), "r");
 731 | -    if (file && boost::filesystem::file_size(pathLog) > 10 * 1000000)
 732 | +    // If debug.log file is more than 10% bigger the RECENT_DEBUG_HISTORY_SIZE

MarcoFalke commented at 1:50 AM on January 25, 2017:

s/the/than/, maybe? Or even better, don't mention the name of the constant and just write. If debug.log exceeds the target size by 10% or more, trim it to the target size. or similar. (It is not like the code here is hard to understand and needs excessive developer documentation)

re-ACK otherwise.

laanwj commented at 9:26 AM on January 26, 2017: member

Incidentally I was looking into file_size and fseek to see whether I needed to have the 10% buffer, and it didn't look to me from the internets that the negative fseek from SEEK_END was necessarily a supported operation?

I'm 100% sure it is. I can't find it explicitly in the man page, but it is the only thing SEEK_END is useful for, you'd hardly ever want to use it with a positive offset (one exception to that is to create sparse files). Generally it is used for file formats that have a directory structure at the end such as ZIP.

laanwj merged this on Feb 1, 2017

laanwj closed this on Feb 1, 2017

laanwj referenced this in commit 77bd8c4cab on Feb 1, 2017

laanwj commented at 10:42 AM on February 1, 2017: member

This seemed ready for merge. @MarcoFalke's last comments seem to be small nits that could be done as a separate patch if necessary.

DrahtBot locked this on Sep 8, 2021