Skip to content

Commit

Permalink
[Jaeger-Agent] Use RawConn.Control to get fd instead of Fd() (#4449)
Browse files Browse the repository at this point in the history
<!--
Please delete this comment before posting.

We appreciate your contribution to the Jaeger project! πŸ‘‹πŸŽ‰

Before creating a pull request, please make sure:
- Your PR is solving one problem
- You have read the guide for contributing
- See
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING.md
- You signed all your commits (otherwise we won't be able to merge the
PR)
- See
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md#certificate-of-origin---sign-your-work
- You added unit tests for the new functionality
- You mention in the PR description which issue it is addressing, e.g.
"Resolves #123"
-->

## Which problem is this PR solving?
Resolves #4448

## Short description of the changes
Jaeger agent gets stuck when closing with SocketBufferSize set. This is
because `Close()` of `net.UDPConn` will be blocked if `Fd()` is used to
get the file descriptor.
Use `RawConn.Control` instead to get fd to set the socket buffer.

Same issue was discussed here: golang/go#29277
The fix refers to here: brucespang/go-tcpinfo#3

Signed-off-by: Chen Xu <[email protected]>
  • Loading branch information
ChenX1993 authored May 11, 2023
1 parent 5c3c20d commit d6b4482
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 3 deletions.
30 changes: 30 additions & 0 deletions cmd/agent/app/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,36 @@ func TestStartStopRace(t *testing.T) {
t.Fatal("Expecting server exit log")
}

func TestStartStopWithSocketBufferSet(t *testing.T) {
resetDefaultPrometheusRegistry()
cfg := Builder{
Processors: []ProcessorConfiguration{
{
Model: jaegerModel,
Protocol: compactProtocol,
Workers: 1,
Server: ServerConfiguration{
HostPort: "127.0.0.1:0",
SocketBufferSize: 10,
},
},
},
}
mBldr := &metricsbuilder.Builder{HTTPRoute: "/metrics", Backend: "prometheus"}
metricsFactory, err := mBldr.CreateMetricsFactory("jaeger")
mFactory := fork.New("internal", metrics.NullFactory, metricsFactory)
require.NoError(t, err)
agent, err := cfg.CreateAgent(fakeCollectorProxy{}, zap.NewNop(), mFactory)
require.NoError(t, err)

if err := agent.Run(); err != nil {
t.Fatalf("error from agent.Run(): %s", err)
}

t.Log("stopping agent")
agent.Stop()
}

func resetDefaultPrometheusRegistry() {
// Create and assign a new Prometheus Registerer/Gatherer for each test
registry := prometheus.NewRegistry()
Expand Down
18 changes: 15 additions & 3 deletions cmd/agent/app/servers/thriftudp/socket_buffer.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,27 @@
package thriftudp

import (
"fmt"
"net"
"syscall"
)

func setSocketBuffer(conn *net.UDPConn, bufferSize int) error {
file, err := conn.File()
rawConn, err := conn.SyscallConn()
if err != nil {
return err
return fmt.Errorf("failed to get raw connection: %w", err)
}

return syscall.SetsockoptInt(int(file.Fd()), syscall.SOL_SOCKET, syscall.SO_RCVBUF, bufferSize)
var syscallErr error
controlErr := rawConn.Control(func(fd uintptr) {
syscallErr = syscall.SetsockoptInt(int(fd), syscall.SOL_SOCKET, syscall.SO_RCVBUF, bufferSize)
})
if controlErr != nil {
return fmt.Errorf("rawconn control failed: %w", controlErr)
}
if syscallErr != nil {
return fmt.Errorf("syscall failed: %w", syscallErr)
}

return nil
}

0 comments on commit d6b4482

Please sign in to comment.