Bug #15717

getListenAddr behavior with multiple instances on same host

Added by Peter Amstutz 6 months ago. Updated 3 months ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Target version:
Start date:
10/15/2019
Due date:
% Done:

100%

Estimated time:
(Total: 0.00 h)
Story points:
-
Release relationship:
Auto

Description

Setting the listen port for keepstore through the legacy config doesn't seem to work, it seems like instead it randomly picks a port to bind to from InternalURLs and if it is in use, it fails, and (in arvbox) it goes into a crash-restart loop until it happens to picks the 2nd port that's still available

Looking at the code, it just adds the legacy "Listen" to InternalURLs, for that to do the right thing, the keepstore InternalURLs needs to be empty to start with.

In getListenAddr in lib/service/cmd.go, it has different behavior depending on the text of the error, in particular, if there are two ports defined for the same host (eg 25107 and 25108) it will fail if it can't bind 25107 and not try to bind 25108.

        } else if strings.Contains(err.Error(), "cannot assign requested address") {
            continue
        } else if strings.Contains(err.Error(), "address already in use") {
            return url, err

Subtasks

Task #15718: Review 15717-svc-select-portResolvedPeter Amstutz

Associated revisions

Revision 6e0e8f65
Added by Peter Amstutz 6 months ago

Merge branch '15717-svc-select-port' refs #15717

Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <>

History

#1 Updated by Peter Amstutz 6 months ago

  • Description updated (diff)

#2 Updated by Tom Clegg 6 months ago

Seems like it would work if "address already in use" didn't fail right away, but instead
  • continue trying other entries
  • if all entries fail, and one or more errors were "address already in use", then report "address(es) already in use" as the error -- rather than the generic "not configured for this host" error that's returned by the current code when all fail but none were handled by that "address already in use" condition

#3 Updated by Peter Amstutz 6 months ago

  • Assigned To set to Peter Amstutz

#5 Updated by Tom Clegg 6 months ago

LGTM, thanks.

(nit -- you can call len() and append to a nil slice, so "var errors []string" might be a better habit for slices that might not need to be allocated at all)

#6 Updated by Peter Amstutz 6 months ago

  • Status changed from New to Resolved

#7 Updated by Peter Amstutz 3 months ago

  • Release set to 22

Also available in: Atom PDF