Enable by default the compression support

[a-noni-mousse]

High Level Overview of Change

Enable link compression by default.

Context of Change

The server can save bandwidth by compressing its p2p communications at the cost of greater CPU usage. Servers that have link compression enabled will automatically compress communications with peers that also have link compression enabled.

In rippled.cfg, you can enable compression with:

[compression]
true

Use false to disable compression. Prior to this PR, false is the default.

Restart the server software. After the restart, your server automatically uses link compression with other peers that also have link compression enabled.

Link compression is not currently mentioned in rippled-example.cfg, but it should be.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Refactor (non-breaking change that only restructures code)
• Tests (You added tests for code that already exists, or your new feature included in this PR)
• Documentation Updates
• Release

[intelliot]

Note: Protocol message compression support was added in #3287

Ping: @gregtatcam @nbougalis @seelabs @HowardHinnant

[gregtatcam]

I'm curious if there is a stats available on the compression adaption rate?

[gregtatcam]

LGTM

[a-noni-mousse]

I do not know about stats of compression sorry! Perhaps Ripple can check from their r.ripple clusters?

[intelliot]

is a stats available on the compression adaption rate?

@xzhaous could you check into this?

[alloynetworks]

I would prefer more testing before making it as default. There could be memory and cpu increases that may affect some nodes. I will enable it on a portion of the zaphod hubs and monitor (currently only a couple have it enabled). Recommend that Ripple also enable compression similarly, if not already done.

[a-noni-mousse]

Have you seen any memory change from this @alloynetworks and @intelliot ?? Making the compressor obligatory will help to reduce the memory usage because now server must make two copies of every message if one other peer server has the compression.

[a-noni-mousse]

Are there any updates @alloynetworks and @intelliot???

[alloynetworks]

Are there any updates @alloynetworks and @intelliot???

I don’t see any performance issues on my nodes. But for reference these are all high end servers with a minimum of 128GB RAM. I don’t know if others also performed tests on different hardware specs.

[a-noni-mousse]

This should merge or i should make patch to remove compression code from code.

[gregtatcam]

I ran some analysis on how many nodes support the compression. First, I crawled the network and got 702 nodes. Out of 702, I can connect to 75 nodes via the protocol. The majority of other nodes either time out to connect or respond with the service is unavailable. 50 nodes out of the 75 connected have the compression enabled. The vast majority of the 75 nodes are hubs; i.e. have a large number of connected peers. We can then assume that the compression adaption by the hubs is 66%. I think, given the adaption rate, it makes sense to turn the compression on by default. It is of course natural for hubs to have the compression enabled as they could save the most on the bandwidth. But to have a significant effect on the network, majority of the nodes have to have the compression enabled.

[ckeshava]

@gregtatcam How did you query the servers if they are using the compression support? Does rippled expose any APIs for such queries or are you using an indirect method to detect compression support?

[ckeshava]

@a-noni-mousse this looks good to me. I'm not able to build this branch locally, but that is due to a problem in my build system. The CI appears to be happy on github 👍

Question:
Despite this change, even if one of the validators on my UNL has compression disabled, then I will need to keep two copies (compressed and uncompressed version) for every message right?

For this change to reap benefits, all of the validators on my UNL need to have compression enabled. (My understanding is that a validator only communicates with the other peers on the UNL, hence those peers which are not on its UNL do not affect the communication complexity of a validator.)

[gregtatcam]

@gregtatcam How did you query the servers if they are using the compression support? Does rippled expose any APIs for such queries or are you using an indirect method to detect compression support?

I sent the protocol handshake, which requested the compression enabled. The response indicates if the compression is supported by the connected peer.

[ckeshava]

@gregory Tsipenyuk ***@***.***> Ok, thanks for the clarification. Regards, Keshava

…

On Wed, Aug 9, 2023 at 11:42 AM Gregory Tsipenyuk ***@***.***> wrote: @gregtatcam <https://urldefense.com/v3/__https://github.com/gregtatcam__;!!PZTMFYE!_WMt0dijPncxc7697YOipVeCYsQYLUY_5vDyCMU-5tJ4gyoQNUQ9V-tqmobxPq2NLBcD6fVFIR98lWKINys0d_hs3Q$> How did you query the servers if they are using the compression support? Does rippled expose any APIs for such queries or are you using an indirect method to detect compression support? I sent the protocol handshake, which requested the compression enabled. The response indicates if the compression is supported by the connected peer. — Reply to this email directly, view it on GitHub <https://urldefense.com/v3/__https://github.com/XRPLF/rippled/pull/4387*issuecomment-1671949449__;Iw!!PZTMFYE!_WMt0dijPncxc7697YOipVeCYsQYLUY_5vDyCMU-5tJ4gyoQNUQ9V-tqmobxPq2NLBcD6fVFIR98lWKINyup5WXKSQ$>, or unsubscribe <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AFB4TNPGU5Q2EKLE3JXGHUDXUPK2XANCNFSM6AAAAAAT3SZLT4__;!!PZTMFYE!_WMt0dijPncxc7697YOipVeCYsQYLUY_5vDyCMU-5tJ4gyoQNUQ9V-tqmobxPq2NLBcD6fVFIR98lWKINytl4U6R-A$> . You are receiving this because your review was requested.Message ID: ***@***.***>

[intelliot]

note: link compression is documented here - https://xrpl.org/enable-link-compression.html

but it is not mentioned in rippled-example.cfg - https://github.com/XRPLF/rippled/blob/develop/cfg/rippled-example.cfg

[intelliot]

@ckeshava - can you look at the code (and if possible, write a unit test) to confirm that even after the change in this PR, compression can still be fully disabled by adding the following to rippled.cfg?

[compression]
false

[ckeshava]

hello @intelliot ,
I discussed the possibility of unit tests with @gregtatcam . He has already written compatibility unit tests here:

rippled/src/test/overlay/compression_test.cpp

Line 483 in a948203

auto handshake = [&](int outboundEnable, int inboundEnable) {

. These handshake tests ensure that if two peers want to communicate with each other, they need to have identical compression settings.

Secondly, @gregtatcam has collected statistics about the bandwidth, CPU and memory consumption with the different compression settings. I can repeat the same experiment and verify the values.

I have a question for @manojsdoshi and his team. Do you have integration tests where the validators have different compression settings? I believe such tests are more reliable in ensuring that the code doesn't break. The ad-hoc experiments above serve as sanity checks, I'd prefer to include these tests in a regular pipeline.

[intelliot]

Internal tracker: RPFC-108

[intelliot]

Note: currently blocked by perf sign off. Low priority.

[sophiax851]

We are trying to get to the low priority ones in the next two weeks. Thanks for the reminder Elliot.

…

On Fri, May 31, 2024 at 11:19 AM Elliot Lee ***@***.***> wrote: Note: currently blocked by perf sign off. Low priority. — Reply to this email directly, view it on GitHub <https://urldefense.com/v3/__https://github.com/XRPLF/rippled/pull/4387*issuecomment-2142766065__;Iw!!PZTMFYE!8qh0ofZY5FCyV8mOe-JeP5WnhiYcbi0-ci89-q0FV6O8grQ5YblLQpjF9Fg48Z0b7vrylafCoNeE6FFWTSQE$>, or unsubscribe <https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AZKCD24WLYGMHJQP5ICDQDTZFC5MZAVCNFSM6AAAAAAT3SZLT6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBSG43DMMBWGU__;!!PZTMFYE!8qh0ofZY5FCyV8mOe-JeP5WnhiYcbi0-ci89-q0FV6O8grQ5YblLQpjF9Fg48Z0b7vrylafCoNeE6JZphhXA$> . You are receiving this because your review was requested.Message ID: ***@***.***>

[q73zhao]

The comparison testing is done against 2.2.0-rc3 between compression enabled by default and without compression.

Without the compression, the transaction tps is around 282.6. With the compression, the transaction tps is around 264.46, which is around 6% deduction. I did not observe any cpu usage increase but some memory increase when the compression is enabled by default. Since there is a small dip, I would recommend not to enable the compression by default feature.

[gregtatcam]

The comparison testing is done against 2.2.0-rc3 between compression enabled by default and without compression.

Without the compression, the transaction tps is around 282.6. With the compression, the transaction tps is around 264.46, which is around 6% deduction. I did not observe any cpu usage increase but some memory increase when the compression is enabled by default. Since there is a small dip, I would recommend not to enable the compression by default feature.

Was the measurement done over one run or over multiple runs?

[q73zhao]

The comparison testing is done against 2.2.0-rc3 between compression enabled by default and without compression.
Without the compression, the transaction tps is around 282.6. With the compression, the transaction tps is around 264.46, which is around 6% deduction. I did not observe any cpu usage increase but some memory increase when the compression is enabled by default. Since there is a small dip, I would recommend not to enable the compression by default feature.

Was the measurement done over one run or over multiple runs?

Hi @gregtatcam It is done over multiple runs of 1 hour test. I did not observe significant cpu increase but around 10% of memory increase

[sophiax851]

Looks like we can't merge this PR due to the impact to the network/validators. We only tested on an internal network with limited nodes, the impact could be more when the # of peers increases.

[intelliot]

Closing due to:

The comparison testing is done against 2.2.0-rc3 between compression enabled by default and without compression.

Without the compression, the transaction tps is around 282.6. With the compression, the transaction tps is around 264.46, which is around 6% deduction. I did not observe any cpu usage increase but some memory increase when the compression is enabled by default. Since there is a small dip, I would recommend not to enable the compression by default feature.

Feel free to re-open (or open a new PR) in the future if anything changes.