Issue 88470043: state/api: save the connection as the first addr

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line
<Enter> :	respond to / edit current comment
d :	mark current comment as done

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s or <Ctrl> + Enter :	save comment
<Esc> :	cancel edit

Issue 88470043: state/api: save the connection as the first addr

Can't Edit
Can't Publish+Mail
Start Review

Created:
10 years ago by jameinel

Modified:
10 years ago

Reviewers:
axw, mp+216091, rog

Visibility:
Public.

Description

state/api: save the connection as the first addr When we connect to the API, we eventually will try all addresses listed. We also will always save the value that we managed to connect with. This change addresses bug #1308487, which has us move the address we successfully connected to as the first server and address we should try next time. This doesn't quite give us a priority, because if we ever fail this one, we'll go back to the default sort order except for one entry. However, I think it still helps. In my testing, I had to bump up the DialAddressInterval all the way up to 500ms before we really wouldn't try most of the addresses. This is because the time here *includes* the TLS handshake (though not the Login round trip). I would be happy to see the value bumped up a bit, but I don't have a solid feeling for what is a great value there. But since we always move the successful connection to the front, we can expect that even if we wait a while to fall back, that will be the uncommon case, as whatever address succeeded in the past is the most likely to succeed the next time. https://code.launchpad.net/~jameinel/juju-core/api-caches-connected-to-1308487/+merge/216091 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 2

Created: 10 years ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Stats (+190 lines, -3 lines)			Patch
A	[revision details]	View	1 chunk	+2 lines, -0 lines	0 comments	Download
M	state/api/export_test.go	View	1 chunk	+1 line, -0 lines	0 comments	Download
M	state/api/state.go	View	2 chunks	+23 lines, -3 lines	2 comments	Download
M	state/api/state_test.go	View	2 chunks	+164 lines, -0 lines	0 comments	Download

Messages

Total messages: 3

Expand All Messages | Collapse All Messages

rog

10 years ago (2014-04-17 17:39:53 UTC) #3

LGTM with a couple of minor suggestions.

https://codereview.appspot.com/88470043/diff/1/state/api/state.go
File state/api/state.go (right):

https://codereview.appspot.com/88470043/diff/1/state/api/state.go#newcode52
state/api/state.go:52: server := servers[serverIndex]
This can be simplified and made more obviously correct by using copy, I think:

server := servers[serverIndex]
hostPort := server[addrIndex]
copy(server[1:addrIndex+1], server[0:addrIndex])
server[0] = hostPort
copy(servers[1:serverIndex+1], servers[0:serverIndex])
servers[0] = server

https://codereview.appspot.com/88470043/diff/1/state/api/state.go#newcode67
state/api/state.go:67: // there.
// If the address is found, it moves it to the start
// of the servers slice, so that it will be used
// with high priority next time.

Expand All Messages | Collapse All Messages