Opened 3 years ago
Closed 3 years ago
#2293 closed enhancement (fixed)
Switch To CachedIteratorCollection in PeerState
Reported by: | slumlord | Owned by: | zzz |
---|---|---|---|
Priority: | maintenance | Milestone: | 0.9.37 |
Component: | router/transport | Version: | 0.9.35 |
Keywords: | cachediteratorcollection, peerstate, udp | Cc: | zzz, Zlatin Balevsky |
Parent Tickets: | Sensitive: | no |
Description
As per ongoing discussion, I have been testing using CachedIteratorCollection
instead of a List
in PeerState
I have 2 test runs of 24 hours each - one with a stock 0.9.35
router and another from the i2p.i2p.slumlord
branch with the CachedIteratorCollection
class and appropriate changes to PeerState
to make use of this class.
I focused on 2 main statistics: udp.sendFailed
& udp.sendAggressiveFailed
0.9.35
:
udp.sendAggressiveFailed: How many volleys was a packet sent before we gave up ➟ 10 min rate: Average: 11.0; Highest average: 10.999. There were 11 events in this period. The period ended 2 min ago. (Average event count: 16.821; Events in peak period: 46) ➡ Graph Data - Graph Event Count ➡ Lifetime average value: 11.0 (2,624 events) udp.sendFailed: How many sends a failed message was pushed ➟ 10 min rate: Average: 1.289; Highest average: 1.145. There were 1,225 events in this period. The period ended 2 min ago. (Average event count: 485.519; Events in peak period: 3485) ➡ Graph Data - Graph Event Count ➡ Lifetime average value: 1.541 (75,741 events)
0.9.35-22-rc-sl
udp.sendAggressiveFailed: How many volleys was a packet sent before we gave up ➟ 10 min rate: Average: 10.999; Highest average: 11.0. There were 47 events in this period. The period ended 8 min ago. (Average event count: 21.5; Events in peak period: 50) ➡ Graph Data - Graph Event Count ➡ Lifetime average value: 11.0 (3,182 events) udp.sendFailed: How many sends a failed message was pushed ➟ 10 min rate: Average: 1.512; Highest average: 1.181. There were 1,239 events in this period. The period ended 8 min ago. (Average event count: 721.554; Events in peak period: 3298) ➡ Graph Data - Graph Event Count ➡ Lifetime average value: 1.512 (106,790 events)
I have the full /stats
page if anyone would like to view other stats.
I have screenshots from the /graphs
page as well, these will be attached.
Notes
- Uptime for both version was at least 24 hours before stats & graphs were collected
- Both versions transferred more than 20 GB of data in each direction
- I allowed for automatic floodfill selection, the router was a floodfill when it was running off the
i2p.i2p.slumlord
branch but not when it was on stock0.9.35
Subtickets
Attachments (5)
Change History (12)
Changed 3 years ago by
Attachment: | udp.sendfailed.png added |
---|
Changed 3 years ago by
Attachment: | udp.sendaggressivefailed.png added |
---|
udp.sendAggressiveFailed graph
comment:1 Changed 3 years ago by
The stats do not immediately show a problem, but before this gets merged I would like to see a more elaborate description of what the latest problem with add(..) was and a unit test which highlights the issue.
comment:2 Changed 3 years ago by
That was my error, introduced when I was trying to keep the code synced between 3 workspaces and adding changes suggested by zzz:
- Alter the
Node
object constructor toNode(prev, element)
fromNode(prev, element, next)
- Simplify
add()
sonewNode
variable declaration/initialization line appears only once
I believe I may have chosen to replicate changes by hand instead of copying the file over from my other workspaces which introduced this error where line 91 read:
final Node<E> newNode = new Node<>(null, element);
instead of
final Node<E> newNode = new Node<>(last, element);
This error is caught by the current set of tests, (which are currently working with JUnit5 - my next task is to tweak the tests to work with the JUnit 4 libraries which we use and check them in so they are run alongside all our other tests) and also results in a lot of NPEs almost immediately.
Also note that I wasn't intending to check this into i2p.i2p
just yet but was stumbling around trying to understand how new branches in monotone are set up - I thought I had made a new branch and committed to that but that turned out to be incorrect.
comment:3 Changed 3 years ago by
As per recent discussions, I will run this test one more time with the following changes:
- Ensure that floodfill mode is off
- Collect statistics once more and calculate the following
- udp.sendFailed per unit of time; per unit of traffic
- udp.sendAggressiveFailed per unit of time; per unit of traffic
comment:4 Changed 3 years ago by
I have run another live network test of the router with the CachedIteratorCollection
class with the following changes to the test:
- Disable floodfill mode
- I decided to run the test for a longer period of time (~72 Hours for stock 0.9.35-23-rc & ~120 Hours for 0.9.35-23-rc-sl
26267b5cf31dfafbdd2c2f8b2e7672afcffaddc9
)
Results
|--------------------------+----------------| | 0.9.35-23-rc | | |--------------------------+----------------| | udp.sendFailed | 563,334 | | udp.sendAggressiveFailed | 13,596 | |--------------------------+----------------| | Bandwidth Usage | 230*2 = 460 GB | | Uptime | ~72 Hours | |--------------------------+----------------|
|------------------------------------------+-----------------| | 0.9.35-23-rc-sl | | | 26267b5cf31dfafbdd2c2f8b2e7672afcffaddc9 | | |------------------------------------------+-----------------| | udp.sendFailed | 1,089,969 | | udp.sendAggressiveFailed | 21,993 | |------------------------------------------+-----------------| | Bandwidth Usage | 600*2 = 1200 GB | | Uptime | ~120 Hours | |------------------------------------------+-----------------|
|------------------------------+-----------------| | 0.9.35-23-rc | 0.9.35-23-rc-sl | |------------------------------+-----------------+--------------| | Per Unit of Uptime | | % Difference | |------------------------------+-----------------+--------------| | udp.sendFailed | | | | 7824.083 | 9083.075 | +16.09 | | udp.sendAggressiveFailed | | | | 188.83 | 183.275 | -2.942 | |------------------------------+-----------------+--------------| | Per Unit of Data Transferred | | | |------------------------------+-----------------+--------------| | udp.sendFailed | | | | 1224.639 | 908.3075 | -25.8305 | | udp.sendAggressiveFailed | | | | 29.556 | 18.3275 | -37.9905 | |------------------------------+-----------------+--------------|
I also calculated one more metric: udp.sendFailed/sendAggressiveFailed per unit of uptime per unit of data transferred:
|--------------------------+-----------------+--------------| | 0.9.35-23-rc | 0.9.35-23-rc-sl | % Difference | |--------------------------+-----------------+--------------| | udp.sendFailed | | | | 17.00887 | 7.56922 | -55.49835 | |--------------------------+-----------------+--------------| | udp.sendAggressiveFailed | | | | 0.4105072 | 0.1527291 | -62.795013 | |--------------------------+-----------------+--------------|
comment:5 Changed 3 years ago by
Based on discussions at the 26th August 2018 meeting, PeerState
in the i2p.i2p
branch has been updated to use CachedIteratorCollection
from revision fedbe16fb20f7b4f432cdbe04cf6f709257f93cf
.
Please test and report any issues found.
Additionally, a set of unit tests for CachedIteratorCollection
has been in the i2p.i2p
branch since revision 5a69b52dc1480ab57715486420b4a731032ff67f
located at router/java/test/junit/net/i2p/router/util/CachedIteratorCollectionTest.java
comment:6 Changed 3 years ago by
Status: | new → testing |
---|
Setting this ticket to testing status. Please test and report and issues.
comment:7 Changed 3 years ago by
Resolution: | → fixed |
---|---|
Status: | testing → closed |
Testing apparently successful
udp.sendFailed graph