From 276088c38633589925cee18af9f3306e5f4642dd Mon Sep 17 00:00:00 2001 From: reesporte Date: Mon, 24 Jan 2022 11:01:56 -0600 Subject: [PATCH 1/2] fix panic on POST /transaction on non-primary node - if we're a non-primary node, redirect to the primary - if non-primary nodes can create transactions now, then the client should not receive an ErrNotPrimaryNode - streamline metrics logic --- api.go | 26 +++++++++++++------------- http/client_test.go | 7 +++---- http/handler.go | 10 ++++++++-- 3 files changed, 24 insertions(+), 19 deletions(-) diff --git a/api.go b/api.go index a373b0a1b..45494b572 100644 --- a/api.go +++ b/api.go @@ -2514,24 +2514,24 @@ func (api *API) StartTransaction(ctx context.Context, id string, timeout time.Du return nil, errors.Wrap(err, "validating api method") } t, err := api.server.StartTransaction(ctx, id, timeout, exclusive, remote) - if exclusive { - switch err { - case nil: + + switch err { + case nil: + if exclusive { api.holder.Stats.Count(MetricExclusiveTransactionRequest, 1, 1.0) - case ErrTransactionExclusive: - api.holder.Stats.Count(MetricExclusiveTransactionBlocked, 1, 1.0) - } - if t.Active { - api.holder.Stats.Count(MetricExclusiveTransactionActive, 1, 1.0) - } - } else { - switch err { - case nil: + } else { api.holder.Stats.Count(MetricTransactionStart, 1, 1.0) - case ErrTransactionExclusive: + } + case ErrTransactionExclusive: + if exclusive { + api.holder.Stats.Count(MetricExclusiveTransactionBlocked, 1, 1.0) + } else { api.holder.Stats.Count(MetricTransactionBlocked, 1, 1.0) } } + if exclusive && t != nil && t.Active { + api.holder.Stats.Count(MetricExclusiveTransactionActive, 1, 1.0) + } return t, err } diff --git a/http/client_test.go b/http/client_test.go index 74c25d7fd..8625c1e2e 100644 --- a/http/client_test.go +++ b/http/client_test.go @@ -14,7 +14,7 @@ import ( "time" "github.com/davecgh/go-spew/spew" - "github.com/molecula/featurebase/v3" + pilosa "github.com/molecula/featurebase/v3" "github.com/molecula/featurebase/v3/http" "github.com/molecula/featurebase/v3/pql" "github.com/molecula/featurebase/v3/server" @@ -1419,12 +1419,11 @@ func TestClientTransactions(t *testing.T) { } // non-primary - if trns, err := client1.StartTransaction(context.Background(), "blah", time.Minute, false); err == nil || - !strings.Contains(err.Error(), pilosa.ErrNodeNotPrimary.Error()) { + if trns, err := client1.StartTransaction(context.Background(), "blah", time.Minute, false); err != nil { t.Fatalf("unexpected error starting on non-primary: %v", err) } else { test.CompareTransactions(t, - nil, + &pilosa.Transaction{ID: "blah", Timeout: time.Minute, Active: true, Exclusive: false, Deadline: expDeadline}, trns) } diff --git a/http/handler.go b/http/handler.go index 1dbf32c68..57aa77de1 100644 --- a/http/handler.go +++ b/http/handler.go @@ -2158,9 +2158,15 @@ func (h *Handler) handlePostTransaction(w http.ResponseWriter, r *http.Request) if !ok { id = reqTrns.ID } - trns, err := h.api.StartTransaction(r.Context(), id, reqTrns.Timeout, reqTrns.Exclusive, false) - h.doTransactionResponse(w, err, trns) + if primary := h.api.PrimaryNode(); h.api.NodeID() == primary.ID { + trns, err := h.api.StartTransaction(r.Context(), id, reqTrns.Timeout, reqTrns.Exclusive, false) + h.doTransactionResponse(w, err, trns) + return + } else { + http.Redirect(w, r, primary.URI.Normalize()+"/transaction/"+id, http.StatusSeeOther) + return + } } func (h *Handler) handlePostFinishTransaction(w http.ResponseWriter, r *http.Request) { From 365789b7911c4efdffbf08519bb23d0d4fb8a05c Mon Sep 17 00:00:00 2001 From: reesporte Date: Mon, 24 Jan 2022 16:25:25 -0600 Subject: [PATCH 2/2] remove unnecessary port bindings --- internal/clustertests/docker-compose-replication2.yml | 6 ------ internal/clustertests/docker-compose.yml | 7 +------ 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/internal/clustertests/docker-compose-replication2.yml b/internal/clustertests/docker-compose-replication2.yml index c44320eef..2fe6992bd 100644 --- a/internal/clustertests/docker-compose-replication2.yml +++ b/internal/clustertests/docker-compose-replication2.yml @@ -5,8 +5,6 @@ services: context: ../.. dockerfile: Dockerfile-clustertests image: ptest - ports: - - "33455:10101" environment: - PILOSA_CLUSTER_COORDINATOR=true - PILOSA_GOSSIP_SEEDS=pilosa1:14000 @@ -20,8 +18,6 @@ services: context: ../.. dockerfile: Dockerfile-clustertests image: ptest - ports: - - "33456:10101" environment: - PILOSA_GOSSIP_SEEDS=pilosa1:14000 - PILOSA_CLUSTER_REPLICAS=2 @@ -34,8 +30,6 @@ services: context: ../.. dockerfile: Dockerfile-clustertests image: ptest - ports: - - "33457:10101" environment: - PILOSA_GOSSIP_SEEDS=pilosa1:14000,pilosa2:14000 - PILOSA_CLUSTER_REPLICAS=2 diff --git a/internal/clustertests/docker-compose.yml b/internal/clustertests/docker-compose.yml index af74a4d30..2508de782 100644 --- a/internal/clustertests/docker-compose.yml +++ b/internal/clustertests/docker-compose.yml @@ -5,8 +5,6 @@ services: context: ../.. dockerfile: Dockerfile-clustertests image: ptest - ports: - - "33455:10101" environment: - PILOSA_NAME=pilosa1 - PILOSA_ETCD_DIR=/root/.etcd @@ -25,8 +23,6 @@ services: context: ../.. dockerfile: Dockerfile-clustertests image: ptest - ports: - - "33456:10101" environment: - PILOSA_NAME=pilosa2 - PILOSA_ETCD_DIR=/root/.etcd @@ -45,8 +41,6 @@ services: context: ../.. dockerfile: Dockerfile-clustertests image: ptest - ports: - - "33457:10101" environment: - PILOSA_NAME=pilosa3 - PILOSA_ETCD_DIR=/root/.etcd @@ -76,5 +70,6 @@ services: - /var/run/docker.sock:/var/run/docker.sock command: - "cd /go/src/github.com/molecula/featurebase/ && go test -mod=vendor -v -count=1 github.com/molecula/featurebase/v3/internal/clustertests" + networks: pilosanet: