fix: set robust limits for a busy blockchain #3150
Conversation
hansieodendaal
changed the title
hansieodendaal
changed the title
| @@ -106,7 +106,7 @@ where T: OutputManagerBackend + 'static | |||
| ); | |||
|
|
|||
| let (sender, receiver) = reply_channel::unbounded(); | |||
| let (publisher, _) = broadcast::channel(200); | |||
| let (publisher, _) = broadcast::channel(3500); | |||
There was a problem hiding this comment.
shouldn't this use the new config?
There was a problem hiding this comment.
It was actually only the transaction event channel size that was made configurable due to stats in the description; do you suggest having more of these configurable?
There was a problem hiding this comment.
Ja, if we are doing it then I think we should do it for both.
There was a problem hiding this comment.
Cool, making all those broadcast channels sizes configurable in the config file.
hansieodendaal
force-pushed
the
ho_set_robust_limits
branch
2 times, most recently
from
c0faa33 to
a6f6a8f
Compare
There was a problem hiding this comment.
I don't think the channel sizes should be in the config file, it really feels like we are bloating the config file with stuff that 99.99% of end users will not care about and the config file is getting insanely bloated.
I say put them in the config object with default values and then its possible to set them via environment variables if we need to.
| output_manager_event_channel_size = 3500 | ||
| # This is the size of the event channel used to communicate base node update events to the wallet. A busy console | ||
| # wallet doing thousands of bulk payments or used for stress testing needs a fairly big size (>300) (default = 50). | ||
| base_node_update_publisher_channel_size = 500 |
There was a problem hiding this comment.
Just for the sake of not bloating this file further. I really think this one can come out as it is not a function of how many transaction's there are its a function of how many times a client makes the call.
|
PR queued successfully. Your position in queue is: 3 |
|
PR is on top of the queue now |
|
PR failed to merge with reason: Some CI status(es) failed |
hansieodendaal
force-pushed
the
ho_set_robust_limits
branch
from
60d1e30 to
81cc076
Compare
|
PR queued successfully. Your position in queue is: 3 |
|
PR is on top of the queue now |
|
PR failed to merge with reason: Some CI status(es) failed |
hansieodendaal
force-pushed
the
ho_set_robust_limits
branch
2 times, most recently
from
47e74f5 to
0a01a3b
Compare
|
PR queued successfully. Your position in queue is: 1 |
|
PR failed to merge with reason: Some CI status(es) failed |
|
PR queued successfully. Your position in queue is: 1 |
|
PR failed to merge with reason: Some CI status(es) failed |
hansieodendaal
force-pushed
the
ho_set_robust_limits
branch
from
20d516c to
0123747
Compare
|
Waiting for approval before queuing |
Updated limits for the base node and wallet that would be robust for a busy blockchain - this was simulated with two sizeable stress tests of 15,000 transactions each.
hansieodendaal
force-pushed
the
ho_set_robust_limits
branch
from
0123747 to
0f894e5
Compare
Description
Updated limits for the base node and wallet that would be robust for a busy blockchain - this was simulated with two sizeable stress tests of 15,000 transactions each, using 3 sender wallets to 3 receiver wallets, with 5,000 to between each wallet pair. Both tests returned a 100% mined as confirmed success ratio. These tests also utilized the new persistent dedup message cache.
The following previous stress test issues were addressed with this PR:
The wallet edit:transaction event broadcast channel was made configurable due to an increase from 200 to 5,500 not being enough as shown by the results below. (The mobile wallet can use the default or its own config settings for memory preservation.)
Motivation and Context
See above
How Has This Been Tested?
System-level stress tests
Checklist:
developmentbranch.