Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
134653: kvserver: deflake TestLeaseQueueLeasePreferencePurgatoryError r=tbg a=tbg

The test sets up an environment in which 40 replicas of interest are
supposed to enter the lease queue purgatory. The test was waiting for
this to happen before proceeding, but was doing so incorrectly: It
checked that the number of replicas in the purgatory matches 40 (as
opposed to checking directly that all ranges of interest had entered
it). Since other ranges could slip in, occasionally the test would
proceed too early, remove the condition that causes ranges to enter the
purgatory, and then find that a few ranges would not be processed (since
they never entered the purgatory in the first place).

This commit fixes this by waiting explicitly for the RangeIDs of
interest to be represented in the lease queue purgatory.

I was able to reproduce the flake in a few minutes on my gceworker via

```
./dev test --count 10000 --stress ./pkg/kv/kvserver \
	--filter TestLeaseQueueLeasePreferencePurgatoryError  -- \
	--jobs 100 --local_resources=cpu=100 --local_resources=memory=HOST_RAM 2>&1
```

This no longer reproduces as of this PR:

```
INFO: Elapsed time: 2091.413s, Critical Path: 166.06s
INFO: 3356 processes: 2 internal, 3354 linux-sandbox.
INFO: Build completed successfully, 3356 total actions
INFO:
//pkg/kv/kvserver:kvserver_test                                          PASSED in 50.3s
  Stats over 3000 runs: max = 50.3s, min = 12.3s, avg = 26.6s, dev = 6.6s
```

Fixes cockroachdb#134578.
Fixes cockroachdb#134768.

The backports will fix but this
Touches cockroachdb#134765.

Epic: none
Release note: None


134744: authserver: fix "use of Span after Finish." in v2 auth r=kyle-a-wong a=kyle-a-wong

fixes a bug where GET api/v2/login results in the error: `use of Span after Finish`. This was happening due to the endpoint re-using the context that is stored in `authenticationV2Server`.

This has been updated to use the request context instead and the context field of `authenticationV2Server` has been removed,

Fixes: cockroachdb#133493
Epic: none
Release note: None

Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Kyle Wong <[email protected]>
  • Loading branch information
3 people committed Nov 11, 2024
3 parents a8f20b1 + 064e2e2 + 16f8c3f commit dcc832a
Show file tree
Hide file tree
Showing 4 changed files with 49 additions and 22 deletions.
12 changes: 8 additions & 4 deletions pkg/kv/kvserver/helpers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,14 @@ func (s *Store) SplitQueuePurgatoryLength() int {
return s.splitQueue.PurgatoryLength()
}

// LeaseQueuePurgatoryLength returns the number of replicas in lease queue
// purgatory.
func (s *Store) LeaseQueuePurgatoryLength() int {
return s.leaseQueue.PurgatoryLength()
// LeaseQueuePurgatory returns a map of RangeIDs representing the purgatory.
func (s *Store) LeaseQueuePurgatory() map[roachpb.RangeID]struct{} {
defer s.leaseQueue.baseQueue.lockProcessing()()
m := make(map[roachpb.RangeID]struct{}, len(s.leaseQueue.baseQueue.mu.purgatory))
for k := range s.leaseQueue.baseQueue.mu.purgatory {
m[k] = struct{}{}
}
return m
}

// SetRaftLogQueueActive enables or disables the raft log queue.
Expand Down
27 changes: 25 additions & 2 deletions pkg/kv/kvserver/lease_queue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package kvserver_test
import (
"context"
"fmt"
"sort"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -160,10 +161,32 @@ func TestLeaseQueueLeasePreferencePurgatoryError(t *testing.T) {
store := tc.GetFirstStoreFromServer(t, 0)
blockTransferTarget.Store(true)
setLeasePreferences(nextPreferredNode)
rangeIDs := func() map[roachpb.RangeID]struct{} {
rows := tdb.Query(t, `SELECT range_id FROM [ SHOW RANGES FROM TABLE t ]`)
var rangeID roachpb.RangeID
m := map[roachpb.RangeID]struct{}{}
for rows.Next() {
require.NoError(t, rows.Scan(&rangeID))
m[rangeID] = struct{}{}
}
require.NoError(t, rows.Err())
return m
}()
require.Len(t, rangeIDs, numRanges)
testutils.SucceedsSoon(t, func() error {
require.NoError(t, store.ForceLeaseQueueProcess())
if purgLen := store.LeaseQueuePurgatoryLength(); purgLen != numRanges {
return errors.Errorf("expected %d in purgatory but got %v", numRanges, purgLen)
purg := store.LeaseQueuePurgatory()
var missing []roachpb.RangeID
for k := range rangeIDs {
if _, ok := purg[k]; !ok {
missing = append(missing, k)
}
}
sort.Slice(missing, func(i, j int) bool {
return missing[i] < missing[j]
})
if len(missing) > 0 {
return errors.Errorf("replicas missing from purgatory: %v", missing)
}
return nil
})
Expand Down
1 change: 0 additions & 1 deletion pkg/server/authserver/api_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ func NewV2Server(
sqlServer: s,
authServer: innerServer,
mux: simpleMux,
ctx: ctx,
basePath: basePath,
}

Expand Down
31 changes: 16 additions & 15 deletions pkg/server/authserver/api_v2_auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ const (
// verification of sessions for regular endpoints happens in authenticationV2Mux,
// not here.
type authenticationV2Server struct {
ctx context.Context
sqlServer SQLServerInterface
authServer *authenticationServer
mux *http.ServeMux
Expand Down Expand Up @@ -127,8 +126,9 @@ func (a *authenticationV2Server) login(w http.ResponseWriter, r *http.Request) {
if r.Method != "POST" {
http.Error(w, "not found", http.StatusNotFound)
}
ctx := r.Context()
if err := r.ParseForm(); err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
}
if r.Form.Get("username") == "" {
Expand All @@ -144,9 +144,9 @@ func (a *authenticationV2Server) login(w http.ResponseWriter, r *http.Request) {
username, _ := username.MakeSQLUsernameFromUserInput(r.Form.Get("username"), username.PurposeValidation)

// Verify the user and check if DB console session could be started.
verified, pwRetrieveFn, err := a.authServer.VerifyUserSessionDBConsole(a.ctx, username)
verified, pwRetrieveFn, err := a.authServer.VerifyUserSessionDBConsole(ctx, username)
if err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
}
if !verified {
Expand All @@ -155,9 +155,9 @@ func (a *authenticationV2Server) login(w http.ResponseWriter, r *http.Request) {
}

// Verify the provided username/password pair.
verified, expired, err := a.authServer.VerifyPasswordDBConsole(a.ctx, username, r.Form.Get("password"), pwRetrieveFn)
verified, expired, err := a.authServer.VerifyPasswordDBConsole(ctx, username, r.Form.Get("password"), pwRetrieveFn)
if err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
}
if expired {
Expand All @@ -169,13 +169,13 @@ func (a *authenticationV2Server) login(w http.ResponseWriter, r *http.Request) {
return
}

session, err := a.createSessionFor(a.ctx, username)
session, err := a.createSessionFor(ctx, username)
if err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
}

apiutil.WriteJSONResponse(r.Context(), w, http.StatusOK, &loginResponse{Session: session})
apiutil.WriteJSONResponse(ctx, w, http.StatusOK, &loginResponse{Session: session})
}

type logoutResponse struct {
Expand Down Expand Up @@ -214,36 +214,37 @@ func (a *authenticationV2Server) logout(w http.ResponseWriter, r *http.Request)
}
var sessionCookie serverpb.SessionCookie
decoded, err := base64.StdEncoding.DecodeString(session)
ctx := r.Context()
if err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
}
if err := protoutil.Unmarshal(decoded, &sessionCookie); err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
}

// Revoke the session.
if n, err := a.sqlServer.InternalExecutor().ExecEx(
a.ctx,
ctx,
"revoke-auth-session",
nil, /* txn */
sessiondata.NodeUserSessionDataOverride,
`UPDATE system.web_sessions SET "revokedAt" = now() WHERE id = $1`,
sessionCookie.ID,
); err != nil {
srverrors.APIV2InternalError(r.Context(), err, w)
srverrors.APIV2InternalError(ctx, err, w)
return
} else if n == 0 {
err := status.Errorf(
codes.InvalidArgument,
"session with id %d nonexistent", sessionCookie.ID)
log.Infof(a.ctx, "%v", err)
log.Infof(ctx, "%v", err)
http.Error(w, "invalid session", http.StatusBadRequest)
return
}

apiutil.WriteJSONResponse(r.Context(), w, http.StatusOK, &logoutResponse{LoggedOut: true})
apiutil.WriteJSONResponse(ctx, w, http.StatusOK, &logoutResponse{LoggedOut: true})
}

func (a *authenticationV2Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
Expand Down

0 comments on commit dcc832a

Please sign in to comment.