[NETCONF-674] Eliminate use of AsyncSshHandlerReader Created: 25/Apr/20 Updated: 05/Jun/20 Resolved: 05/Jun/20 |
|
| Status: | Resolved |
| Project: | netconf |
| Component/s: | netconf |
| Affects Version/s: | None |
| Fix Version/s: | Aluminium, Sodium SR3, Magnesium SR2 |
| Type: | Improvement | Priority: | Medium |
| Reporter: | Robert Varga | Assignee: | Robert Varga |
| Resolution: | Done | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||
| Description |
|
AsyncSshHandlerReader attaches to public client-side interface, issuing asynchronous reads. Each such read requires a buffer into which it is going to be fed. This unfortunate design is a legacy from previous SSH library we used and is not as efficient as we can go. Specifically, we need to use a relatively small buffer, because we can have a large number of devices – hence 2 are going with 2KiB. This means there are really two levels of queueing going on:
Furthermore we have NETCONF message reassembly sitting on top this queueing, which defaults to at most 16 ByteBufs before coalescence. The interplay of these is very unfortunate:
This results in aweful lot of copying for NETCONF message larger than 32KiB, with the hurt being exponential with the size of messages. |
| Comments |
| Comment by Robert Varga [ 25/Apr/20 ] |
|
The solution is a tighter integration with SSHD, where we subclass SshClient to allow us to tap into internal dispatch of it. Specifically we expose a specialized NETCONF subsystem getter, which feeds directly to a target Netty channel. This integration can tag directly to receive path of SSHD, hence it completely eliminates internal queuing as well as fragmentation, resulting in:
This means that the number of chunks in reassembly drops significantly, as we no longer fragment on receive (or at lest not as much, but still NETCONF-571 applies) thus the reassembly buffer can grow to as much as 16x32=512KiB before it is coalesced. |
| Comment by Jamo Luhrsen [ 13/May/20 ] |
|
rovarga I see one patch associated for this jia is there for I'm also wondering why this ticket does not have that patch in it's gerrit reviews section. the commit message |
| Comment by Robert Varga [ 13/May/20 ] |
|
You need to select 'Show All Reviews' in the right-hand '...' menu of Gerrit Reviews. The patches will land in stable/magnesium after Mg SR1, as they are not critical and have landed after the freeze was in effect. |
| Comment by Jamo Luhrsen [ 13/May/20 ] |
| Comment by Robert Varga [ 13/May/20 ] |
|
|
| Comment by Jamo Luhrsen [ 13/May/20 ] |
|
thanks. Gotta be a problem on my end. oh well. |