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

More granular debug #8484

Closed
wants to merge 1 commit into from
Closed

Conversation

rebroad
Copy link
Contributor

@rebroad rebroad commented Aug 8, 2016

This pull request has been shunk down to size with other changes now to be in separate pull requests.

The change request here simply splits moves some of "net" debugging into "net2", "block" and "tx".

This is a cut-down version of #8728 which also seeks to make debug message conform to a style.

Sorry for all the pull requests as a result of breaking this up, but I felt there was too much to it in it's previous format, and therefore was unlikely any of it would be merged.

@rebroad rebroad force-pushed the MoreGranularDebug branch 2 times, most recently from c93416a to cff229f Compare August 8, 2016 03:33
@sipa
Copy link
Member

sipa commented Aug 10, 2016

Concept ACK

@maflcko
Copy link
Member

maflcko commented Aug 10, 2016

Wouldn't need init.cpp (debugCategories) an update as well?

@laanwj
Copy link
Member

laanwj commented Aug 11, 2016

Wouldn't need init.cpp (debugCategories) an update as well?

Yes, needs to be documented in option help.

@rebroad
Copy link
Contributor Author

rebroad commented Aug 21, 2016

ok, will add a commit to update option help... shall I list each debug string e.g. "tx, tx2, tx3"? or is it better to show the level of granularity another way, e.g. "tx{,2,3}" or "tx[1:3]", "tx[1-3]"...? not sure if there's a standard for denoting this sort of thing.

@rebroad rebroad changed the title More granular debug [WIP] More granular debug Aug 26, 2016
@rebroad
Copy link
Contributor Author

rebroad commented Aug 26, 2016

@laanwj init.cpp now updated to show usage

@rebroad rebroad changed the title [WIP] More granular debug More granular debug Aug 26, 2016
@@ -413,7 +413,7 @@ std::string HelpMessage(HelpMessageMode mode)
strUsage += HelpMessageOpt("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT));
strUsage += HelpMessageOpt("-bip9params=deployment:start:end", "Use given start/end times for specified BIP9 deployment (regtest-only)");
}
string debugCategories = "addrman, alert, bench, coindb, db, http, libevent, lock, mempool, mempoolrej, net, proxy, prune, rand, reindex, rpc, selectcoins, tor, zmq"; // Don't translate these and qt below
string debugCategories = "addrman, alert, bench, block, coindb, db, estimatefee{,2}, http, libevent, lock, mempool(,2}, mempoolrej, net{,2}, proxy, prune, rand, reindex, rpc, selectcoins, tx{,2}, tor, zmq"; // Don't translate these and qt below
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add cmpctblock?

Copy link
Member

Choose a reason for hiding this comment

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

μNit: I was wondering if it could help to add a "rule of thumb" somewhere to explain when to use category vs. category2.

Copy link
Contributor Author

@rebroad rebroad Aug 28, 2016

Choose a reason for hiding this comment

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

@MarcoFalke Done. I propose that for future use, "cmpctblock" be restricted to the actual debug's relating to creation/etc of compact blocks, but for the transmission and download that block{,2} etc be used (since "block", "tx" are subsets of "net"). Sound ok?

Regarding rule of thumb, happy to do this (perhaps in a future pull) - best put into init.cpp or into a readme file somewhere?

@rebroad rebroad force-pushed the MoreGranularDebug branch 3 times, most recently from 1ea729d to f3a6fa0 Compare August 28, 2016 09:38
@rebroad
Copy link
Contributor Author

rebroad commented Aug 28, 2016

On a slightly OT note - can someone tell me how one gets notifications when someone comments on a pull request? My currently practice is to scan through my pull requests to see if comments have appeared, but I suspect there may be a better way!!

@rebroad rebroad changed the title More granular debug [WIP] More granular debug Aug 28, 2016
@rebroad rebroad closed this Aug 28, 2016
@sipa
Copy link
Member

sipa commented Aug 28, 2016

@rebroad You already have another pull request from the same branch.

@rebroad rebroad reopened this Aug 29, 2016
@rebroad rebroad force-pushed the MoreGranularDebug branch 3 times, most recently from 4aa39f0 to be9d1f3 Compare August 29, 2016 05:10
@rebroad
Copy link
Contributor Author

rebroad commented Aug 29, 2016

still testing. will re-open when more done.

@sipa
Copy link
Member

sipa commented Aug 29, 2016

Can you please stop opening and closing this PR?

@maflcko
Copy link
Member

maflcko commented Aug 29, 2016

@rebroad Just leave this open. You can do as much testing as you want locally, but be sure to compile and run the tests before you push.

@maflcko maflcko reopened this Aug 29, 2016
@@ -4775,12 +4785,15 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
CBlock block;
if (!ReadBlockFromDisk(block, (*mi).second, consensusParams))
assert(!"cannot load block from disk");
if (inv.type == MSG_BLOCK)
if (inv.type == MSG_BLOCK) {
LogPrint(fRecent ? "block" : "block2", "sending regular %s (%d) to peer=%d\n", inv.ToString(), nHeight, pfrom->id);
Copy link

@n1bor n1bor Aug 29, 2016

Choose a reason for hiding this comment

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

Any reason not to add the size of the block too? Useful to analyse bandwidth usage by block height.
Although that is logged in beginmessage/endmessage already so a nasty bit of awking should be able to join them together.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you mean log the size when sending it or when receiving it, or both?

@paveljanik
Copy link
Contributor

@rebroad rebroad force-pushed the MoreGranularDebug branch 3 times, most recently from 8ae6347 to 1bc8a51 Compare September 14, 2016 16:39
@rebroad rebroad mentioned this pull request Sep 14, 2016
@rebroad rebroad changed the title [WIP] More granular debug More granular debug Sep 14, 2016
@rebroad rebroad closed this Sep 14, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants