diff --git a/api.go b/api.go index 92a1bc1b7..080c1a8e5 100644 --- a/api.go +++ b/api.go @@ -118,16 +118,9 @@ func (api *API) SetAPIOptions(opts ...apiOption) error { // validAPIMethods specifies the api methods that are valid for each // cluster state. var validAPIMethods = map[disco.ClusterState]map[apiMethod]struct{}{ - disco.ClusterStateNormal: appendMap(methodsCommon, methodsNormal), - // Ideally, this would be just `appendMap(methodsCommon, methodsDegraded)`, - // but in an attempt to reduce the influence that state (determined by etcd) - // has on a node under load, this is set to effectively allow all requests - // in a DEGRADED state. - disco.ClusterStateDegraded: appendMap(methodsCommon, methodsNormal), - // Ideally, this would be just `methodsCommon`, but in an attempt to reduce - // the influence that state (determined by etcd) has on a node under load, - // this is set to effectively allow all requests in a DOWN state. - disco.ClusterStateDown: appendMap(methodsCommon, methodsNormal), + disco.ClusterStateNormal: appendMap(methodsCommon, methodsNormal), + disco.ClusterStateDegraded: appendMap(methodsCommon, methodsDegraded), + disco.ClusterStateDown: methodsCommon, } func appendMap(a, b map[apiMethod]struct{}) map[apiMethod]struct{} { @@ -3304,24 +3297,25 @@ var methodsCommon = map[apiMethod]struct{}{ apiState: {}, } -// var methodsDegraded = map[apiMethod]struct{}{ -// apiExportCSV: {}, -// apiFragmentBlockData: {}, -// apiFragmentBlocks: {}, -// apiField: {}, -// apiIndex: {}, -// apiQuery: {}, -// apiRecalculateCaches: {}, -// apiRemoveNode: {}, -// apiShardNodes: {}, -// apiSchema: {}, -// apiViews: {}, -// apiStartTransaction: {}, -// apiFinishTransaction: {}, -// apiTransactions: {}, -// apiGetTransaction: {}, -// apiActiveQueries: {}, -// } +var methodsDegraded = map[apiMethod]struct{}{ + apiExportCSV: {}, + apiFragmentBlockData: {}, + apiFragmentBlocks: {}, + apiField: {}, + apiIndex: {}, + apiQuery: {}, + apiRecalculateCaches: {}, + apiShardNodes: {}, + apiSchema: {}, + apiViews: {}, + apiStartTransaction: {}, + apiFinishTransaction: {}, + apiTransactions: {}, + apiGetTransaction: {}, + apiActiveQueries: {}, + apiPastQueries: {}, + apiPartitionNodes: {}, +} var methodsNormal = map[apiMethod]struct{}{ apiCreateField: {}, diff --git a/server/server_test.go b/server/server_test.go index e01d645ec..a4c5ba3ed 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -543,11 +543,11 @@ func TestClusteringNodesReplica1(t *testing.T) { Query: fmt.Sprintf("Row(%s=1)", fieldName), } - // check for connection refused... this used to check 'shard - // unavailable', but we since made a change to allow queries to - // nodes which the cluster *thinks* are down, but often are - // actually not. - if _, err := cluster.GetPrimary().API.Query(context.Background(), qry); !strings.Contains(err.Error(), "connection refused") { + _, err := cluster.GetPrimary().API.Query(context.Background(), qry) + if err == nil { + t.Fatalf("expected error from cluster with downed node, didn't get an error") + } + if !strings.Contains(err.Error(), "not allowed in state") { t.Fatalf("got unexpected error querying an incomplete cluster: %v", err) } } @@ -603,7 +603,7 @@ func TestClusteringNodesReplica2(t *testing.T) { } if err := others[0].Close(); err != nil { - t.Fatalf("closing third node: %v", err) + t.Fatalf("closing first node: %v", err) } err = cluster.AwaitPrimaryState(disco.ClusterStateDegraded, 30*time.Second) @@ -611,13 +611,24 @@ func TestClusteringNodesReplica2(t *testing.T) { t.Fatalf("after closing first server: %v", err) } - // We no longer support mutations or schema changes when the cluster is in - // state DEGRADED, so this test doesn't apply anymore. - // - // // confirm that cluster keeps accepting queries if replication > 1 - // if _, err := coord.API.CreateIndex(context.Background(), "anewindex", pilosa.IndexOptions{}); err != nil { - // t.Fatalf("got unexpected error creating index: %v", err) - // } + qry := &pilosa.QueryRequest{ + Index: "idx", + Query: fmt.Sprintf("Row(%s=1)", fieldName), + } + + // With only one node down, we expect a query to still work. + resp, err := coord.API.Query(context.Background(), qry) + if err != nil { + t.Fatalf("unexpected error from degraded cluster: %v", err) + } + if len(resp.Results) == 0 { + t.Fatal("got no results") + } + row, ok := resp.Results[0].(*pilosa.Row) + if !ok { + t.Fatalf("expected a *pilosa.Row, but got %T", resp.Results[0]) + } + require.Equal(t, row.Columns(), cols) // confirm that cluster stops accepting queries if 2 nodes fail and replication == 2 if err := others[1].Close(); err != nil { @@ -629,34 +640,12 @@ func TestClusteringNodesReplica2(t *testing.T) { t.Fatalf("after closing second server: %v", err) } - qry := &pilosa.QueryRequest{ - Index: "idx", - Query: fmt.Sprintf("Row(%s=1)", fieldName), + _, err = coord.API.Query(context.Background(), qry) + if err == nil { + t.Fatalf("expected a cluster with two down nodes to reject query") } - - // Because we no longer block queries when the cluster is in state DOWN, - // there are cases where a DOWN cluster can still respond to a query. In - // that case, we want the test to pass. But if the unavailable node(s) cause - // the query to result in an error, we check that it's the error we expect. - resp, err := coord.API.Query(context.Background(), qry) - if err != nil { - // check for connection refused... this used to check 'shard - // unavailable', but we since made a change to allow queries to - // nodes which the cluster *thinks* are down, but often are - // actually not. - if !strings.Contains(err.Error(), "connection refused") { - t.Fatalf("got unexpected error querying an incomplete cluster: %v", err) - } - } else { - if len(resp.Results) == 0 { - t.Fatal("got no results") - } - - row, ok := resp.Results[0].(*pilosa.Row) - if !ok { - t.Fatalf("expected a *pilosa.Row, but got %T", resp.Results[0]) - } - require.Equal(t, row.Columns(), cols) + if !strings.Contains(err.Error(), "not allowed in state") { + t.Fatalf("expected not allowed in state error, got %q", err.Error()) } }