Skip to content

Commit

Permalink
core: fix and add test for per locality hosts
Browse files Browse the repository at this point in the history 
Connected the existing code and added an acceptance roachtest.

closes: 29591

Release note: None
  • Loading branch information
Masha Schneider committed Oct 3, 2018
1 parent e65a612 commit a7b24d6
Show file tree
Hide file tree
Showing 12 changed files with 98 additions and 29 deletions.
4 changes: 3 additions & 1 deletion pkg/cli/cliflags/flags.go
View file
Expand Up @@ -391,9 +391,11 @@ example [::1]:8080 or [fe80::f6f2:::]:8080.`,
Description: `
List of ports to advertise to other CockroachDB nodes for intra-cluster
communication for some locality. This should be specified as a commma
separated list of locality@address. For example:
separated list of locality@address. Addresses can also include ports.
For example:
<PRE>
"region=us-west@127.0.0.1,datacenter=us-west-1b@127.0.0.1"
"region=us-west@127.0.0.1:26257,datacenter=us-west-1b@127.0.0.1:26258"
</PRE>
`,
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/cli/flags.go
View file
Expand Up @@ -541,7 +541,12 @@ func extraServerFlagInit() {
serverHTTPAddr = startCtx.serverListenAddr
}
serverCfg.HTTPAddr = net.JoinHostPort(serverHTTPAddr, serverHTTPPort)
serverCfg.LocalityIPAddresses = localityAdvertiseHosts
for i, addr := range localityAdvertiseHosts {
if !strings.Contains(localityAdvertiseHosts[i].Address.AddressField, ":") {
localityAdvertiseHosts[i].Address.AddressField = net.JoinHostPort(addr.Address.AddressField, serverAdvertisePort)
}
}
serverCfg.LocalityAddresses = localityAdvertiseHosts
}

func extraClientFlagInit() {
Expand Down
9 changes: 5 additions & 4 deletions pkg/cli/flags_test.go
View file
Expand Up @@ -407,8 +407,9 @@ func TestServerConnSettings(t *testing.T) {
{[]string{"start", "--advertise-addr", "192.168.0.111", "--port", "12345"}, ":12345", "192.168.0.111:12345", "[]"},
{[]string{"start", "--advertise-addr", "192.168.0.111", "--advertise-port", "12345"}, ":" + base.DefaultPort, "192.168.0.111:12345", "[]"},
{[]string{"start", "--advertise-addr", "192.168.0.111", "--port", "54321", "--advertise-port", "12345"}, ":54321", "192.168.0.111:12345", "[]"},
{[]string{"start", "--host", "127.0.0.1", "--locality-advertise-addr", "zone=1@235.0.0.5"}, "127.0.0.1:" + base.DefaultPort, "127.0.0.1:" + base.DefaultPort, "[{{tcp 235.0.0.5} zone=1}]"},
{[]string{"start", "--host", "127.0.0.1", "--locality-advertise-addr", "zone=1@235.0.0.5,zone=2@123.0.0.5"}, "127.0.0.1:" + base.DefaultPort, "127.0.0.1:" + base.DefaultPort, "[{{tcp 235.0.0.5} zone=1} {{tcp 123.0.0.5} zone=2}]"},
{[]string{"start", "--host", "127.0.0.1", "--locality-advertise-addr", "zone=1@235.0.0.5"}, "127.0.0.1:" + base.DefaultPort, "127.0.0.1:" + base.DefaultPort, "[{{tcp 235.0.0.5:26257} zone=1}]"},
{[]string{"start", "--host", "127.0.0.1", "--locality-advertise-addr", "zone=1@235.0.0.5,zone=2@123.0.0.5"}, "127.0.0.1:" + base.DefaultPort, "127.0.0.1:" + base.DefaultPort, "[{{tcp 235.0.0.5:26257} zone=1} {{tcp 123.0.0.5:26257} zone=2}]"},
{[]string{"start", "--host", "127.0.0.1", "--locality-advertise-addr", "zone=1@235.0.0.5:1234"}, "127.0.0.1:" + base.DefaultPort, "127.0.0.1:" + base.DefaultPort, "[{{tcp 235.0.0.5:1234} zone=1}]"},
}

for i, td := range testData {
Expand All @@ -427,9 +428,9 @@ func TestServerConnSettings(t *testing.T) {
t.Errorf("%d. serverCfg.AdvertiseAddr expected '%s', but got '%s'. td.args was '%#v'.",
i, td.expectedAdvertiseAddr, serverCfg.AdvertiseAddr, td.args)
}
if td.expLocalityAdvertiseAddr != fmt.Sprintf("%s", serverCfg.LocalityIPAddresses) {
if td.expLocalityAdvertiseAddr != fmt.Sprintf("%s", serverCfg.LocalityAddresses) {
t.Errorf("%d. serverCfg.expLocalityAdvertiseAddr expected '%s', but got '%s'. td.args was '%#v'.",
i, td.expLocalityAdvertiseAddr, serverCfg.LocalityIPAddresses, td.args)
i, td.expLocalityAdvertiseAddr, serverCfg.LocalityAddresses, td.args)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cli/flags_util.go
View file
Expand Up @@ -42,7 +42,7 @@ func (l *localityList) Type() string { return "localityList" }
func (l *localityList) String() string {
string := ""
for _, loc := range []roachpb.LocalityAddress(*l) {
string += loc.LocalityTier.Key + "=" + loc.LocalityTier.Value + ":" + loc.Address.String() + ","
string += loc.LocalityTier.Key + "=" + loc.LocalityTier.Value + "@" + loc.Address.String() + ","
}

return string
Expand Down
1 change: 1 addition & 0 deletions pkg/cmd/roachtest/acceptance.go
View file
Expand Up @@ -45,6 +45,7 @@ func registerAcceptance(r *registry) {
{"gossip/peerings", runGossipPeerings},
{"gossip/restart", runGossipRestart},
{"gossip/restart-node-one", runGossipRestartNodeOne},
{"gossip/locality-address", runCheckLocalityIPAddress},
{"rapid-restart", runRapidRestart},
{"status-server", runStatusServer},
{"version-upgrade", runVersionUpgrade},
Expand Down
52 changes: 52 additions & 0 deletions pkg/cmd/roachtest/gossip.go
View file
Expand Up @@ -405,3 +405,55 @@ SELECT count(replicas)

g.checkConnectedAndFunctional(ctx, t, c)
}

func runCheckLocalityIPAddress(ctx context.Context, t *test, c *cluster) {
c.Put(ctx, cockroach, "./cockroach")

externalIP := c.ExternalIP(ctx, c.Range(1, c.nodes))

for i := 1; i <= c.nodes; i++ {
if local {
externalIP[i-1] = "localhost"
}
extAddr := externalIP[i-1]

c.Start(ctx, c.Node(i), startArgs("--racks=1",
fmt.Sprintf("--args=--locality-advertise-addr=rack=0@%s", extAddr)))
}

rowCount := 0

for i := 1; i <= c.nodes; i++ {
db := c.Conn(ctx, 1)
defer db.Close()

rows, err := db.Query(
`SELECT node_id, advertise_address FROM crdb_internal.gossip_nodes`,
)
if err != nil {
t.Fatal(err)
}

for rows.Next() {
rowCount++
var nodeID int
var advertiseAddress string
if err := rows.Scan(&nodeID, &advertiseAddress); err != nil {
t.Fatal(err)
}

if local {
if !strings.Contains(advertiseAddress, "localhost") {
t.Fatal("Expected connect address to contain localhost")
}
} else if c.ExternalAddr(ctx, c.Node(nodeID))[0] != advertiseAddress {
t.Fatalf("Connection address is %s but expected %s",
advertiseAddress, c.ExternalAddr(ctx, c.Node(nodeID))[0])
}
}
}
if rowCount <= 0 {
t.Fatal("No results for " +
"SELECT node_id, advertise_address FROM crdb_internal.gossip_nodes")
}
}
4 changes: 2 additions & 2 deletions pkg/server/config.go
View file
Expand Up @@ -228,9 +228,9 @@ type Config struct {
// Locality is a description of the topography of the server.
Locality roachpb.Locality

// LocalityIPAddresses contains private IP addresses the can only be accessed
// LocalityAddresses contains private IP addresses the can only be accessed
// in the corresponding locality.
LocalityIPAddresses []roachpb.LocalityAddress
LocalityAddresses []roachpb.LocalityAddress

// EventLogEnabled is a switch which enables recording into cockroach's SQL
// event log tables. These tables record transactional events about changes
Expand Down
15 changes: 8 additions & 7 deletions pkg/server/node.go
View file
Expand Up @@ -429,13 +429,14 @@ func (n *Node) start(

n.startedAt = n.storeCfg.Clock.Now().WallTime
n.Descriptor = roachpb.NodeDescriptor{
NodeID: nodeID,
Address: util.MakeUnresolvedAddr(addr.Network(), addr.String()),
Attrs: attrs,
Locality: locality,
ServerVersion: n.storeCfg.Settings.Version.ServerVersion,
BuildTag: build.GetInfo().Tag,
StartedAt: n.startedAt,
NodeID: nodeID,
Address: util.MakeUnresolvedAddr(addr.Network(), addr.String()),
Attrs: attrs,
Locality: locality,
LocalityAddress: localityAddress,
ServerVersion: n.storeCfg.Settings.Version.ServerVersion,
BuildTag: build.GetInfo().Tag,
StartedAt: n.startedAt,
}
// Invoke any passed in nodeDescriptorCallback as soon as it's available, to
// ensure that other components (currently the DistSQLPlanner) are initialized
Expand Down
2 changes: 1 addition & 1 deletion pkg/server/server.go
View file
Expand Up @@ -1443,7 +1443,7 @@ func (s *Server) Start(ctx context.Context) error {
s.cfg.NodeAttributes,
s.cfg.Locality,
cv,
s.cfg.LocalityIPAddresses,
s.cfg.LocalityAddresses,
s.execCfg.DistSQLPlanner.SetNodeDesc,
); err != nil {
return err
Expand Down
27 changes: 17 additions & 10 deletions pkg/sql/crdb_internal.go
View file
Expand Up @@ -1740,17 +1740,18 @@ CREATE TABLE crdb_internal.zones (
var crdbInternalGossipNodesTable = virtualSchemaTable{
schema: `
CREATE TABLE crdb_internal.gossip_nodes (
node_id INT NOT NULL,
network STRING NOT NULL,
address STRING NOT NULL,
attrs JSON NOT NULL,
locality JSON NOT NULL,
server_version STRING NOT NULL,
build_tag STRING NOT NULL,
started_at TIMESTAMP NOT NULL,
node_id INT NOT NULL,
network STRING NOT NULL,
address STRING NOT NULL,
advertise_address STRING NOT NULL,
attrs JSON NOT NULL,
locality JSON NOT NULL,
server_version STRING NOT NULL,
build_tag STRING NOT NULL,
started_at TIMESTAMP NOT NULL,
is_live BOOL NOT NULL,
ranges INT NOT NULL,
leases INT NOT NULL
ranges INT NOT NULL,
leases INT NOT NULL
)
`,
populate: func(ctx context.Context, p *planner, _ *DatabaseDescriptor, addRow func(...tree.Datum) error) error {
Expand Down Expand Up @@ -1824,10 +1825,16 @@ CREATE TABLE crdb_internal.gossip_nodes (
locality.Add(t.Key, json.FromString(t.Value))
}

addr, err := g.GetNodeIDAddress(d.NodeID)
if err != nil {
return err
}

if err := addRow(
tree.NewDInt(tree.DInt(d.NodeID)),
tree.NewDString(d.Address.NetworkField),
tree.NewDString(d.Address.AddressField),
tree.NewDString(addr.String()),
tree.NewDJSON(attrs.Build()),
tree.NewDJSON(locality.Build()),
tree.NewDString(d.ServerVersion.String()),
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/logictest/testdata/planner_test/explain
View file
Expand Up @@ -224,7 +224,7 @@ sort · ·
├── render · ·
│ └── filter · ·
│ └── values · ·
│ size 20 columns, 915 rows
│ size 20 columns, 916 rows
└── render · ·
└── filter · ·
└── values · ·
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/opt/exec/execbuilder/testdata/explain
View file
Expand Up @@ -215,7 +215,7 @@ sort · ·
├── render · ·
│ └── filter · ·
│ └── values · ·
│ size 20 columns, 915 rows
│ size 20 columns, 916 rows
└── render · ·
└── filter · ·
└── values · ·
Expand Down

0 comments on commit a7b24d6

Please sign in to comment.