fix: Make neqo pass amplificationlimit QNS test
#1875
Conversation
|
One test not passing, see #1490 (comment) |
Failed Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
All resultsSucceeded Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
Unsupported Interop TestsQUIC Interop Runner, client vs. server neqo-latest as client
neqo-latest as server
|
|
@martinthomson @KershawChang if any of you understands the timer code, I'd appreciate some help here. The It's straightforward to reproduce: |
I think we need to implement what Martin suggested to do in this comment. |
Then it dies here And when I take that out, too, it spins forever. |
|
Making this a draft until the test failure is fixed. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1875 +/- ##
=======================================
Coverage 94.99% 94.99%
=======================================
Files 112 112
Lines 36366 36371 +5
=======================================
+ Hits 34546 34552 +6
+ Misses 1820 1819 -1 ☔ View full report in Codecov by Sentry. |
Benchmark resultsPerformance differences relative to 2e419cc. coalesce_acked_from_zero 1+1 entries: Change within noise threshold. time: [189.65 ns 190.16 ns 190.73 ns]
change: [-1.3971% -0.9570% -0.5150%] (p = 0.00 < 0.05)
coalesce_acked_from_zero 3+1 entries: Change within noise threshold. time: [232.27 ns 232.84 ns 233.47 ns]
change: [-0.9548% -0.5922% -0.2226%] (p = 0.00 < 0.05)
coalesce_acked_from_zero 10+1 entries: Change within noise threshold. time: [232.27 ns 232.85 ns 233.58 ns]
change: [-1.0816% -0.6282% -0.1029%] (p = 0.01 < 0.05)
coalesce_acked_from_zero 1000+1 entries: Change within noise threshold. time: [216.18 ns 216.43 ns 216.68 ns]
change: [-1.3019% -0.6518% -0.0209%] (p = 0.04 < 0.05)
RxStreamOrderer::inbound_frame(): No change in performance detected. time: [119.57 ms 119.74 ms 119.99 ms]
change: [-0.2703% -0.0082% +0.2759%] (p = 0.96 > 0.05)
transfer/Run multiple transfers with varying seeds: Change within noise threshold. time: [55.861 ms 58.819 ms 61.812 ms]
thrpt: [64.713 MiB/s 68.005 MiB/s 71.607 MiB/s]
change:
time: [+0.8767% +8.8633% +17.466%] (p = 0.03 < 0.05)
thrpt: [-14.869% -8.1417% -0.8691%]
transfer/Run multiple transfers with the same seed: No change in performance detected. time: [63.853 ms 70.430 ms 76.973 ms]
thrpt: [51.966 MiB/s 56.794 MiB/s 62.644 MiB/s]
change:
time: [-19.554% -8.8575% +3.1440%] (p = 0.14 > 0.05)
thrpt: [-3.0481% +9.7183% +24.307%]
1-conn/1-100mb-resp (aka. Download)/client: No change in performance detected. time: [287.17 ms 293.54 ms 299.50 ms]
thrpt: [333.89 MiB/s 340.67 MiB/s 348.22 MiB/s]
change:
time: [-1.6402% +0.7881% +3.3389%] (p = 0.56 > 0.05)
thrpt: [-3.2310% -0.7820% +1.6675%]
1-conn/10_000-parallel-1b-resp (aka. RPS)/client: No change in performance detected. time: [406.71 ms 410.20 ms 413.77 ms]
thrpt: [24.168 Kelem/s 24.378 Kelem/s 24.588 Kelem/s]
change:
time: [-2.2923% -1.0746% +0.1154%] (p = 0.08 > 0.05)
thrpt: [-0.1153% +1.0863% +2.3461%]
1-conn/1-1b-resp (aka. HPS)/client: No change in performance detected. time: [67.949 ms 68.263 ms 68.623 ms]
thrpt: [14.572 elem/s 14.649 elem/s 14.717 elem/s]
change:
time: [-0.7625% -0.0222% +0.6877%] (p = 0.95 > 0.05)
thrpt: [-0.6830% +0.0223% +0.7683%]
Client/server transfer resultsTransfer of 33554432 bytes over loopback.
|
|
Removing the early return makes it not pass the |
|
This again is waiting for a fix to the |
|
I've tried to give this another look and I found that the problem might be about when the function neqo/neqo-transport/src/path.rs Line 948 in e2fc9b0 Before this change, the server was able to update the RTT to the value set here With this change, the server can only update the RTT to a small value: Unfortunately, I still can't figure out the correct way to fix this. |
I don't see this log line when I run the test for this PR? (I do see the one for the
What seems to happen is that |
larseggert
force-pushed
the
fix-1183
branch
2 times, most recently
from
2fb43fc
to
f574643
Compare
|
Some QNS L1/C1 tests are failing because #1981 is needed. |
* Cancel runs after 20 min, and report that that happened * Build NSS from source, to make SSLKEYLOGFILE work * Shorten retention for some artifacts * Run interop server at `debug` log level Factored out of mozilla#1875, which was a convenient test case.
Co-authored-by: Martin Thomson <mt@lowentropy.net> Signed-off-by: Lars Eggert <lars@eggert.org>
|
So this seems to work, in the sense that there are no more But maybe there are also more It's difficult to say something definitive here, since we have had these occur randomly all along, and they may also not be due to bugs on our end. |
Fixes #1183