feat: Option for enabled TPC_QUICKACK for sync process#22635
feat: Option for enabled TPC_QUICKACK for sync process#22635
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## main #22635 +/- ##
=========================================
Coverage 75.99% 76.00%
- Complexity 23548 23549 +1
=========================================
Files 2550 2550
Lines 94344 94352 +8
Branches 10060 10060
=========================================
+ Hits 71699 71708 +9
+ Misses 18968 18964 -4
- Partials 3677 3680 +3
... and 11 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| this.configuration = configuration; | ||
| this.quickAck = configuration.getConfigData(SocketConfig.class).quickAck(); |
There was a problem hiding this comment.
can we consolidate both config and quickACk and store
this.socketConfig = configuration.getConfigData(SocketConfig.class); instead?
i see many of these cases where we store both the configuration instance and one particular value of the configuration as a field and its is confusing IMO
Signed-off-by: Artur Biesiadowski <artur.biesiadowski@swirldslabs.com>
Signed-off-by: Artur Biesiadowski <artur.biesiadowski@swirldslabs.com>
08831dd to
b8f41df
Compare
Description:
Possibility for enabling TCP_QUICKACK for sync.
Please read https://catonmat.net/tcp-quickack for context.
This PR has it disabled by default, as it is hard to test and does NOT work on Mac (it shouldn't break anything, just won't give any benefits).
Hope is that it can reduce the latency of sync slightly in some environments. If it has no visible benefits after enabling on linux, it is probably not worth to include it.
Related issue(s):
Fixes #22189
Checklist