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

add retry for socket scanning with delay #3794

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add retry for socket scanning with delay #3794

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85a0b67
add retry for socket scanning with delay
fliepeltje Jul 29, 2024
6685082
Update internal/command/deploy/machines_deploymachinesapp.go
fliepeltje Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 26 additions & 20 deletions internal/command/deploy/machines_deploymachinesapp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1119,28 +1119,34 @@ func (md *machineDeployment) warnAboutIncorrectListenAddress(ctx context.Context
}

var foundSockets int
for _, proc := range processes {
for _, ls := range proc.ListenSockets {
foundSockets += 1

host, portStr, err := net.SplitHostPort(ls.Address)
if err != nil {
continue
}
port, err := strconv.Atoi(portStr)
if err != nil {
continue
}

ip := net.ParseIP(host)

// We don't know VM's internal ipv4 which is also a valid address to bind to.
// Let's assume that whoever binds to a non-loopback address knows what they are doing.
// If we expose this address to flyctl later, we can revisit this logic.
if !ip.IsLoopback() {
delete(tcpServices, port)
retries := 1
for retries >= 0 {
for _, proc := range processes {
for _, ls := range proc.ListenSockets {
foundSockets += 1
host, portStr, err := net.SplitHostPort(ls.Address)
if err != nil {
continue
}
port, err := strconv.Atoi(portStr)
if err != nil {
continue
}
ip := net.ParseIP(host)
// We don't know VM's internal ipv4 which is also a valid address to bind to.
// Let's assume that whoever binds to a non-loopback address knows what they are doing.
// If we expose this address to flyctl later, we can revisit this logic.
if !ip.IsLoopback() {
delete(tcpServices, port)
}
}
}
if len(tcpServices) == 0 {
break
} else {
retries -= 1
time.Sleep(3 * time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Also! @fliepeltje, something to ponder about: would you consider reducing the sleep time to 1 second for each retry?

  1. Pro: This will reduce wait time in case the services becomes reachable earlier than 3 seconds
  2. Con: You'll have to increase the number of retries though! Since the intended logic is that most services are reachable by 3 seconds, when reducing the sleep time to 1 second, the retries have to be increased to reach that 3 second mark. So that's a drawback, since that would increase requests made to the app right.

Also this is a very experimental question, with the aim of taking into consideration the wait time of users. But if 3 seconds is not that long, and is not any different from waiting 1 second, then I guess this comment don't need to considered even.

}
}

// This can either mean that nothing is listening or that VM is running old init that doesn't expose
Expand Down
Loading