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.

Sign up for GitHub

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

Jump to bottom

Upgrade pion/webrtc to v4 #3098

Merged
merged 3 commits into from
Dec 25, 2024
Merged

Upgrade pion/webrtc to v4 #3098

merged 3 commits into from
Dec 25, 2024

Conversation

badgooooor
Copy link
Contributor

Description

p-shahi
p-shahi previously requested changes Dec 16, 2024
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@p-shahi p-shahi linked an issue Dec 16, 2024 that may be closed by this pull request
webrtc: upgrade pion/webrtc to v4 #3072
Closed
@badgooooor badgooooor requested a review from p-shahi December 16, 2024 15:23
@p-shahi
Copy link
Member

p-shahi commented Dec 16, 2024 •
edited
Loading

This webrtc v4 upgrade seems to make TestManyConnections racy
(There's no issue when I run 100 iterations of this test on master)

I was trying to narrow down what the problem was so I ran 100 iterations (go test ./p2p/transport/webrtc -run TestManyConnections -count=1) with the following values

const numListeners = 2
const dialersPerListener = 2
const connsPerDialer = 1

without any failure.

After increasing numListeners and dialersPerListener to 3 but keeping connections per dialer to 1:

const numListeners = 3
const dialersPerListener = 3
const connsPerDialer = 1

I can repro the issue seen in CI

❯ for i in {1..100}; do
  echo "Running iteration $i..."
  go test ./p2p/transport/webrtc -run TestManyConnections -count=1
done
Running iteration 1...
--- FAIL: TestManyConnections (0.03s)
    transport_test.go:982: dial failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:1006: failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:968: listener failed to accept conneciton: listener closed
    transport_test.go:968: listener failed to accept conneciton: listener closed
FAIL
FAIL	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.371s
FAIL
Running iteration 2...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.585s
Running iteration 3...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.370s
Running iteration 4...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.583s
Running iteration 5...
--- FAIL: TestManyConnections (0.03s)
    transport_test.go:982: dial failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:1006: failed: failed to secure outbound connection: error reading handshake message: proto: cannot parse invalid wire-format data
    transport_test.go:968: listener failed to accept conneciton: listener closed
    transport_test.go:968: listener failed to accept conneciton: listener closed
FAIL
FAIL	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.369s
FAIL
Running iteration 6...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.372s
Running iteration 7...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.372s
Running iteration 8...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.382s
Running iteration 9...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.575s
Running iteration 10...
ok  	github.com/libp2p/go-libp2p/p2p/transport/webrtc	0.594s

cc @sukunrt @MarcoPolo

@badgooooor
Copy link
Contributor Author

I tried downsize the test parameters to something like this. It is still a bit flaky.

const numListeners = 3
const dialersPerListener = 3
const connsPerDialer = 6

Log:

Running tool: /usr/local/go/bin/go test -timeout 30s -run ^TestManyConnections$ github.com/libp2p/go-libp2p/p2p/transport/webrtc

=== RUN   TestManyConnections
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  0
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  1
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  2
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  3
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  4
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  5
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  6
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  7
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  8
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  9
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  10
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  11
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  12
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  13
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  14
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  15
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  16
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  17
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  18
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  19
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  20
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  21
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  22
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  23
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  24
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  25
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  26
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  27
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  28
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  29
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  30
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  31
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  32
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  33
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  34
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  35
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  36
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  37
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  38
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  39
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  40
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  41
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  42
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  43
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  44
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  45
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  46
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  47
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  48
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  49
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  50
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  51
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  52
    /go-libp2p/p2p/transport/webrtc/transport_test.go:1004: completed conn:  53
--- PASS: TestManyConnections (0.24s)
PASS
ok      github.com/libp2p/go-libp2p/p2p/transport/webrtc        (cached)

cc. @p-shahi @sukunrt

@sukunrt
Copy link
Member

sukunrt commented Dec 23, 2024

Thanks @badgooooor. Looks like an issue in pion.

blocked on: pion/webrtc#2976

@sukunrt sukunrt self-assigned this Dec 24, 2024
@sukunrt sukunrt force-pushed the chore/upgrade-pion-webrtc branch from e14b7fa to 78e750c Compare December 25, 2024 11:57
@sukunrt sukunrt force-pushed the chore/upgrade-pion-webrtc branch from 78e750c to e4c0a07 Compare December 25, 2024 12:19
sukunrt
sukunrt approved these changes Dec 25, 2024
View reviewed changes
Copy link
Member

@sukunrt sukunrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @badgooooor ❤️

@sukunrt sukunrt merged commit 8f6d98d into libp2p:master Dec 25, 2024
9 checks passed
2color added a commit that referenced this pull request Jan 24, 2025
@2color
Merge remote-tracking branch 'origin/marco/bring-go-nat-home' into de…
1bd76eb 
…bug-nat-port-mapping

* origin/marco/bring-go-nat-home: (75 commits)
  go mod tidy
  reference internal package for mockgen
  mod tidy test-plans
  Make changes to internal nat library
  ci: move to actions/upload-artifact@v4 (#3152)
  tcpreuse: fix rcmgr accounting when tcp metrics are enabled (#3142)
  fix(net/nat): data race problem of `extAddr` (#3140)
  test: fix failing test (#3141)
  quicreuse: make it possible to use an application-constructed quic.Transport (#3122)
  nat: ignore mapping if external port is 0 (#3094)
  tcpreuse: error on using tcpreuse with pnet (#3129)
  chore: Update contribution guidelines (#3134)
  tcp: fix metrics test build directive (#3052)
  webrtc: upgrade pion/webrtc to v4 (#3098)
  ci: get back on the main release track of release checker (#3117)
  webtransport: fix docstring comment for getCurrentBucketStartTime
  chore: release v0.38.1 (#3114)
  fix(httpauth): Correctly handle concurrent requests on server (#3111)
  ci: Install specific protoc version when generating protobufs (#3112)
  fix(autorelay): Move relayFinder peer disconnect cleanup to separate goroutine (#3105)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sukunrt sukunrt sukunrt approved these changes

@p-shahi p-shahi Awaiting requested review from p-shahi

Assignees

@sukunrt sukunrt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

webrtc: upgrade pion/webrtc to v4
3 participants
@badgooooor @p-shahi @sukunrt