code review 5486078: net: RFC: API proposal for DialPlan (issue 5486078)

116 views
Skip to first unread message

brad...@golang.org

unread,
Dec 14, 2011, 6:14:07 PM12/14/11
to golan...@googlegroups.com, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reviewers: golang-dev_googlegroups.com,

Message:
Hello golan...@googlegroups.com (cc: golan...@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg


Description:
net: RFC: API proposal for DialPlan

This is an API proposal that Russ and I sketched out
awhile back and recently dusted off.

Let's discuss the API before any implementation.

Issue 240

Please review this at http://codereview.appspot.com/5486078/

Affected files:
M src/pkg/net/dial.go


Index: src/pkg/net/dial.go
===================================================================
--- a/src/pkg/net/dial.go
+++ b/src/pkg/net/dial.go
@@ -4,6 +4,44 @@

package net

+import (
+ "time"
+)
+
+// A DialPlan represents the strategy for connecting to a remote
+// address.
+type DialPlan struct {
+ // Attempts lists
+ Attempts []DialAttempt
+}
+
+// A DialAttempt describes one attempt of a DialPlan.
+type DialAttempt struct {
+ // Remote is the destination to connect to.
+ Remote Addr
+
+ // Local optionally specifies a local address to bind to.
+ Local Addr
+
+ // Timeout, if non-zero, specifies the maximum amount
+ // of time to wait for the connection to complete.
+ Timeout time.Duration
+
+ // SoftTimeout, if non-zero, specifies the amount of
+ // time to wait for this attempt to complete before
+ // starting the next attempt and racing them.
+ // See http://tools.ietf.org/html/draft-ietf-v6ops-happy-eyeballs-06
+ SoftTimeout time.Duration
+}
+
+func (p *DialPlan) Dial() (Conn, error) {
+ panic("TODO")
+}
+
+func NewDialPlan(net, addr string) *DialPlan {
+ panic("TODO")
+}
+
func resolveNetAddr(op, net, addr string) (a Addr, err error) {
if addr == "" {
return nil, &OpError{op, net, nil, errMissingAddress}
@@ -65,6 +103,12 @@
return
}

+// DialTimeout is like Dial, but returns ErrTimeout after timeout if a
+// connection isn’t established.
+func DialTimeout(net, addr string, timeout time.Duration) (c Conn, err
error) {
+ panic("TODO")
+}
+
// Listen announces on the local network address laddr.
// The network string net must be a stream-oriented
// network: "tcp", "tcp4", "tcp6", or "unix", or "unixpacket".


David Symonds

unread,
Dec 14, 2011, 6:47:07 PM12/14/11
to brad...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
Can you describe what DialPlan/DialAttempt are for? When would I use
them? Is it meant to be some kind of multi-address auto-failover
dialing mechanism?


Dave.

Sameer Ajmani

unread,
Dec 14, 2011, 7:51:34 PM12/14/11
to golang-dev

On Dec 14, 6:14 pm, bradf...@golang.org wrote:
> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golan...@googlegroups.com (cc: golan...@googlegroups.com),
>

> I'd like you to review this change tohttps://go.googlecode.com/hg


>
> Description:
> net: RFC: API proposal for DialPlan
>
> This is an API proposal that Russ and I sketched out
> awhile back and recently dusted off.
>
> Let's discuss the API before any implementation.
>
> Issue 240
>

> Please review this athttp://codereview.appspot.com/5486078/

> +       // Seehttp://tools.ietf.org/html/draft-ietf-v6ops-happy-eyeballs-06


> +       SoftTimeout time.Duration
> +}
> +
> +func (p *DialPlan) Dial() (Conn, error) {
> +       panic("TODO")
> +}
> +
> +func NewDialPlan(net, addr string) *DialPlan {
> +       panic("TODO")
> +}

Does NewDialPlan generate a default DialPlan for the given net and
addr? For example, if addr is "www.google.com", I'm guessing
NewDialPlan will resolve that DNS name into several IP addresses, each
one of which becomes a DialAttempt in the DialPlan. The SoftTimeout
and Timeout for each are set to some default values. The user can
modify these before invoking Dial to execute the plan. Fancier dial
plans might implement some load balancing policy, though it's unclear
which parameter of NewDialPlan encodes this policy. (Presumably this
same pattern could be applied to other replicated operations, e.g.,
connecting/reading/writing to the nearest available database replica.)

"DialAttempt" sounds like a completed attempt, not a planned attempt.
Perhaps "DialTarget" would be a better name.

Should NewDialPlan take a timeout parameter and ensure that each
DialAttempt.Timeout matches this? Alternatively, DialPlan.SetTimeout
could set the timeout for all planned DialAttempts.

Is "SoftTimeout" a well-known term? I've called this
"FailoverTimeout" in similar interfaces for talking to replicated
storage backends; that seems to convey more meaning.

David Crawshaw

unread,
Dec 14, 2011, 11:26:51 PM12/14/11
to brad...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com

If I have multiple targets for an RPC, I want to keep dialing until
the RPC completes. This API only gets to the starting line.

Russ Cox

unread,
Dec 15, 2011, 11:45:40 AM12/15/11
to David Crawshaw, brad...@golang.org, golan...@googlegroups.com, re...@codereview-hr.appspotmail.com
On Wed, Dec 14, 2011 at 23:26, David Crawshaw <craw...@google.com> wrote:
> If I have multiple targets for an RPC, I want to keep dialing until
> the RPC completes. This API only gets to the starting line.

I think that's debatable. You want to find a server that
will take the RPC. Whether you want to have multiple
servers working on your RPC racing to be the one who
gets done first is a separate issue. In any event, that's
not what this CL is or needs to be about. This is just
about trying to connect.

Russ

Reply all
Reply to author
Forward
0 new messages