Skip to content

Conversation

theuni
Copy link
Member

@theuni theuni commented Sep 13, 2016

This was broken by 63cafa6.

Note that while this fixes the settings, it doesn't fix the actual usage of
-maxuploadtarget completely, as there is currently a bug in the
nOptimisticBytesWritten accounting that causes a delayed response if the target
is reached. That bug will be addressed separately.

Thanks to @morcos for reporting.

@jonasschnelli
Copy link
Contributor

utACK 86ce4ec

@@ -72,6 +72,8 @@ static const size_t SETASKFOR_MAX_SZ = 2 * MAX_INV_SZ;
static const unsigned int DEFAULT_MAX_PEER_CONNECTIONS = 125;
/** The default for -maxuploadtarget. 0 = Unlimited */
static const uint64_t DEFAULT_MAX_UPLOAD_TARGET = 0;
/** The default for -maxuploadtarget. 0 = Unlimited */
Copy link
Member

Choose a reason for hiding this comment

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

I think the comment is wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoops, c/p and forgot to actually fill it in. Will fix.

@jonasschnelli
Copy link
Contributor

Could get affected/obsolete by #8712

@maflcko
Copy link
Member

maflcko commented Sep 13, 2016

Right, needs a rebase after #8712 probably.

@theuni
Copy link
Member Author

theuni commented Sep 13, 2016

Roger, will rebase after merge.

@@ -2362,11 +2363,7 @@ void CConnman::RecordBytesSent(uint64_t bytes)
void CConnman::SetMaxOutboundTarget(uint64_t limit)
{
LOCK(cs_totalBytesSent);
uint64_t recommendedMinimum = (nMaxOutboundTimeframe / 600) * MAX_BLOCK_SERIALIZED_SIZE;
Copy link
Member

Choose a reason for hiding this comment

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

Needs rebase to solve silent merge conflict.

This was broken by 63cafa6.

Note that while this fixes the settings, it doesn't fix the actual usage of
-maxuploadtarget completely, as there is currently a bug in the
nOptimisticBytesWritten accounting that causes a delayed response if the target
is reached. That bug will be addressed separately.
@theuni
Copy link
Member Author

theuni commented Sep 14, 2016

Rebased after #8712 and fixed the comment.

@maflcko
Copy link
Member

maflcko commented Sep 16, 2016

Concept ACK. Can confirm that this together with #8708 fixes the test failure.

Copy link
Member

@laanwj laanwj left a comment

Choose a reason for hiding this comment

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

utACK f3552da

@laanwj laanwj merged commit f3552da into bitcoin:master Sep 19, 2016
laanwj added a commit that referenced this pull request Sep 19, 2016
f3552da net: fix maxuploadtarget setting (Cory Fields)
codablock pushed a commit to codablock/dash that referenced this pull request Jan 11, 2018
f3552da net: fix maxuploadtarget setting (Cory Fields)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
f3552da net: fix maxuploadtarget setting (Cory Fields)
@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.

4 participants