Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

transport tests: many streams and lots of data #2296

Merged
merged 7 commits into from
Jun 15, 2023

Conversation

MarcoPolo
Copy link
Collaborator

Related to #2194.

There is a failure with mplex that I haven't spent time yet debugging. If it's not obvious, I'd suggest we fix this in parallel with the webrtc work. Meaning merge this as is and have an issue to fix it soon.

@MarcoPolo
Copy link
Collaborator Author

Windows stalls with webtransport:

2023-05-16T19:50:01.2122591Z === RUN   TestManyBigPings/WebTransport
2023-05-16T19:50:01.2122972Z panic: test timed out after 10m0s
2023-05-16T19:50:01.2123256Z running tests:

@MarcoPolo
Copy link
Collaborator Author

MarcoPolo commented May 19, 2023

@MarcoPolo MarcoPolo closed this May 19, 2023
@BigLep
Copy link
Contributor

BigLep commented May 19, 2023

@MarcoPolo : I saw you closed this. Is that because there is a replacement PR coming?

@MarcoPolo MarcoPolo reopened this May 19, 2023
@MarcoPolo
Copy link
Collaborator Author

@MarcoPolo : I saw you closed this. Is that because there is a replacement PR coming?

Fat fingered thanks for the flag.

@MarcoPolo
Copy link
Collaborator Author

I'd lean towards skipping windows tests for now and creating a tracking issue so as not to block WebRTC integration. Thoughts @marten-seemann ?

I'm also spinning up a Windows VM to try to investigate this a bit as well.

@MarcoPolo
Copy link
Collaborator Author

Hmm windows qlog does not look good. Large latency in the RTT measurement and the session takes ~300ms with a lot of gaps of inactivity on a recording with just 2 streams.

Here's the qlogs:
qlogs.zip

@marten-seemann could you take a look if anything obvious jumps out to you?

p2p/test/transport/transport_test.go Outdated Show resolved Hide resolved
p2p/test/transport/transport_test.go Outdated Show resolved Hide resolved
p2p/test/transport/transport_test.go Outdated Show resolved Hide resolved
@MarcoPolo MarcoPolo requested a review from marten-seemann June 7, 2023 02:19
p2p/test/transport/transport_test.go Outdated Show resolved Hide resolved
@marten-seemann marten-seemann changed the title Transport Integration tests: Many streams and lots of data transport tests: many streams and lots of data Jun 15, 2023
@marten-seemann marten-seemann merged commit 91f34d4 into master Jun 15, 2023
@MarcoPolo MarcoPolo mentioned this pull request Jul 14, 2023
29 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants