Opened 9 months ago

Closed 7 months 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, zab
Parent Tickets:

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 stock 0.9.35

Subtickets

Attachments (5)

udp.sendfailed.png (28.4 KB) - added by slumlord 9 months ago.
udp.sendFailed graph
udp.sendaggressivefailed.png (30.3 KB) - added by slumlord 9 months ago.
udp.sendAggressiveFailed graph
memoryused.png (40.0 KB) - added by slumlord 9 months ago.
MemoryUsed? graph
bandwidth.png (52.6 KB) - added by slumlord 9 months ago.
Bandwidth graph
activepeers.png (38.1 KB) - added by slumlord 9 months ago.
ActivePeers? graph

Download all attachments as: .zip

Change History (12)

Changed 9 months ago by slumlord

udp.sendFailed graph

Changed 9 months ago by slumlord

udp.sendAggressiveFailed graph

Changed 9 months ago by slumlord

MemoryUsed? graph

Changed 9 months ago by slumlord

Bandwidth graph

Changed 9 months ago by slumlord

ActivePeers? graph

comment:1 Changed 9 months ago by zab

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 9 months ago by slumlord

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 to Node(prev, element) fromNode(prev, element, next)
  • Simplify add() so newNode 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 our currenty JUnit 4 libraries and check them in so they are run alongside all our other tests) and also results in a lot of NPEs almost immediately.

Version 0, edited 9 months ago by slumlord (next)

comment:3 Changed 8 months ago by slumlord

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 8 months ago by slumlord

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 8 months ago by slumlord

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 8 months ago by slumlord

  • Status changed from new to testing

Setting this ticket to testing status. Please test and report and issues.

comment:7 Changed 7 months ago by zzz

  • Resolution set to fixed
  • Status changed from testing to closed

Testing apparently successful

Note: See TracTickets for help on using tickets.