From d9408473c58e970d99a2a37f4f2414235f616965 Mon Sep 17 00:00:00 2001 From: Marek Siarkowicz Date: Fri, 4 Aug 2023 16:27:16 +0200 Subject: [PATCH] server: Unify arguments for mvcc methods Signed-off-by: Marek Siarkowicz --- server/etcdserver/apply/apply.go | 10 +++---- server/etcdserver/apply/apply_auth.go | 12 ++++----- server/etcdserver/apply/apply_auth_test.go | 4 +-- server/etcdserver/apply/corrupt.go | 8 +++--- server/etcdserver/apply/uber_applier.go | 4 +-- server/etcdserver/txn/txn.go | 31 +++++++++++++++------- server/etcdserver/v3_server.go | 2 +- 7 files changed, 41 insertions(+), 30 deletions(-) diff --git a/server/etcdserver/apply/apply.go b/server/etcdserver/apply/apply.go index f5180b0c0..7b6963d79 100644 --- a/server/etcdserver/apply/apply.go +++ b/server/etcdserver/apply/apply.go @@ -72,8 +72,8 @@ type applierV3 interface { Apply(ctx context.Context, r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3, applyFunc applyFunc) *Result Put(ctx context.Context, p *pb.PutRequest) (*pb.PutResponse, *traceutil.Trace, error) - Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, error) - DeleteRange(dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) + Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, *traceutil.Trace, error) + DeleteRange(ctx context.Context, dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, *traceutil.Trace, error) Txn(ctx context.Context, rt *pb.TxnRequest) (*pb.TxnResponse, *traceutil.Trace, error) Compaction(compaction *pb.CompactionRequest) (*pb.CompactionResponse, <-chan struct{}, *traceutil.Trace, error) @@ -161,11 +161,11 @@ func (a *applierV3backend) Put(ctx context.Context, p *pb.PutRequest) (resp *pb. return mvcctxn.Put(ctx, a.lg, a.lessor, a.kv, p) } -func (a *applierV3backend) DeleteRange(dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) { - return mvcctxn.DeleteRange(a.kv, dr) +func (a *applierV3backend) DeleteRange(ctx context.Context, dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, *traceutil.Trace, error) { + return mvcctxn.DeleteRange(ctx, a.lg, a.kv, dr) } -func (a *applierV3backend) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, error) { +func (a *applierV3backend) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, *traceutil.Trace, error) { return mvcctxn.Range(ctx, a.lg, a.kv, r) } diff --git a/server/etcdserver/apply/apply_auth.go b/server/etcdserver/apply/apply_auth.go index 51cf5ad86..7321f4224 100644 --- a/server/etcdserver/apply/apply_auth.go +++ b/server/etcdserver/apply/apply_auth.go @@ -86,25 +86,25 @@ func (aa *authApplierV3) Put(ctx context.Context, r *pb.PutRequest) (*pb.PutResp return aa.applierV3.Put(ctx, r) } -func (aa *authApplierV3) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, error) { +func (aa *authApplierV3) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, *traceutil.Trace, error) { if err := aa.as.IsRangePermitted(&aa.authInfo, r.Key, r.RangeEnd); err != nil { - return nil, err + return nil, nil, err } return aa.applierV3.Range(ctx, r) } -func (aa *authApplierV3) DeleteRange(r *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) { +func (aa *authApplierV3) DeleteRange(ctx context.Context, r *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, *traceutil.Trace, error) { if err := aa.as.IsDeleteRangePermitted(&aa.authInfo, r.Key, r.RangeEnd); err != nil { - return nil, err + return nil, nil, err } if r.PrevKv { err := aa.as.IsRangePermitted(&aa.authInfo, r.Key, r.RangeEnd) if err != nil { - return nil, err + return nil, nil, err } } - return aa.applierV3.DeleteRange(r) + return aa.applierV3.DeleteRange(ctx, r) } func (aa *authApplierV3) Txn(ctx context.Context, rt *pb.TxnRequest) (*pb.TxnResponse, *traceutil.Trace, error) { diff --git a/server/etcdserver/apply/apply_auth_test.go b/server/etcdserver/apply/apply_auth_test.go index 0e2252f3a..644933d72 100644 --- a/server/etcdserver/apply/apply_auth_test.go +++ b/server/etcdserver/apply/apply_auth_test.go @@ -532,7 +532,7 @@ func TestAuthApplierV3_Range(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { setAuthInfo(authApplier, tc.userName) - _, err := authApplier.Range(ctx, tc.request) + _, _, err := authApplier.Range(ctx, tc.request) require.Equalf(t, tc.expectError, err, "Range returned unexpected error (or lack thereof), expected: %v, got: %v", tc.expectError, err) }) } @@ -596,7 +596,7 @@ func TestAuthApplierV3_DeleteRange(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { setAuthInfo(authApplier, tc.userName) - _, err := authApplier.DeleteRange(tc.request) + _, _, err := authApplier.DeleteRange(context.Background(), tc.request) require.Equalf(t, tc.expectError, err, "Range returned unexpected error (or lack thereof), expected: %v, got: %v", tc.expectError, err) }) } diff --git a/server/etcdserver/apply/corrupt.go b/server/etcdserver/apply/corrupt.go index e04d28c6f..73527ebdb 100644 --- a/server/etcdserver/apply/corrupt.go +++ b/server/etcdserver/apply/corrupt.go @@ -32,12 +32,12 @@ func (a *applierV3Corrupt) Put(_ context.Context, _ *pb.PutRequest) (*pb.PutResp return nil, nil, errors.ErrCorrupt } -func (a *applierV3Corrupt) Range(_ context.Context, _ *pb.RangeRequest) (*pb.RangeResponse, error) { - return nil, errors.ErrCorrupt +func (a *applierV3Corrupt) Range(_ context.Context, _ *pb.RangeRequest) (*pb.RangeResponse, *traceutil.Trace, error) { + return nil, nil, errors.ErrCorrupt } -func (a *applierV3Corrupt) DeleteRange(_ *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) { - return nil, errors.ErrCorrupt +func (a *applierV3Corrupt) DeleteRange(_ context.Context, _ *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, *traceutil.Trace, error) { + return nil, nil, errors.ErrCorrupt } func (a *applierV3Corrupt) Txn(_ context.Context, _ *pb.TxnRequest) (*pb.TxnResponse, *traceutil.Trace, error) { diff --git a/server/etcdserver/apply/uber_applier.go b/server/etcdserver/apply/uber_applier.go index 72e591ed7..d3184e696 100644 --- a/server/etcdserver/apply/uber_applier.go +++ b/server/etcdserver/apply/uber_applier.go @@ -153,13 +153,13 @@ func (a *uberApplier) dispatch(ctx context.Context, r *pb.InternalRaftRequest, s switch { case r.Range != nil: op = "Range" - ar.Resp, ar.Err = a.applyV3.Range(ctx, r.Range) + ar.Resp, ar.Trace, ar.Err = a.applyV3.Range(ctx, r.Range) case r.Put != nil: op = "Put" ar.Resp, ar.Trace, ar.Err = a.applyV3.Put(ctx, r.Put) case r.DeleteRange != nil: op = "DeleteRange" - ar.Resp, ar.Err = a.applyV3.DeleteRange(r.DeleteRange) + ar.Resp, ar.Trace, ar.Err = a.applyV3.DeleteRange(ctx, r.DeleteRange) case r.Txn != nil: op = "Txn" ar.Resp, ar.Trace, ar.Err = a.applyV3.Txn(ctx, r.Txn) diff --git a/server/etcdserver/txn/txn.go b/server/etcdserver/txn/txn.go index 6070389fd..cc96062be 100644 --- a/server/etcdserver/txn/txn.go +++ b/server/etcdserver/txn/txn.go @@ -93,20 +93,30 @@ func put(ctx context.Context, txnWrite mvcc.TxnWrite, p *pb.PutRequest) (resp *p return resp, nil } -func DeleteRange(kv mvcc.KV, dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) { - txnWrite := kv.Write(traceutil.TODO()) +func DeleteRange(ctx context.Context, lg *zap.Logger, kv mvcc.KV, dr *pb.DeleteRangeRequest) (resp *pb.DeleteRangeResponse, trace *traceutil.Trace, err error) { + trace = traceutil.Get(ctx) + // create delete tracing if the trace in context is empty + if trace.IsEmpty() { + trace = traceutil.New("delete_range", + lg, + traceutil.Field{Key: "key", Value: string(dr.Key)}, + traceutil.Field{Key: "range_end", Value: string(dr.RangeEnd)}, + ) + ctx = context.WithValue(ctx, traceutil.TraceKey, trace) + } + txnWrite := kv.Write(trace) defer txnWrite.End() - resp, err := deleteRange(txnWrite, dr) - return resp, err + resp, err = deleteRange(ctx, txnWrite, dr) + return resp, trace, err } -func deleteRange(txnWrite mvcc.TxnWrite, dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) { +func deleteRange(ctx context.Context, txnWrite mvcc.TxnWrite, dr *pb.DeleteRangeRequest) (*pb.DeleteRangeResponse, error) { resp := &pb.DeleteRangeResponse{} resp.Header = &pb.ResponseHeader{} end := mkGteRange(dr.RangeEnd) if dr.PrevKv { - rr, err := txnWrite.Range(context.TODO(), dr.Key, end, mvcc.RangeOptions{}) + rr, err := txnWrite.Range(ctx, dr.Key, end, mvcc.RangeOptions{}) if err != nil { return nil, err } @@ -122,15 +132,16 @@ func deleteRange(txnWrite mvcc.TxnWrite, dr *pb.DeleteRangeRequest) (*pb.DeleteR return resp, nil } -func Range(ctx context.Context, lg *zap.Logger, kv mvcc.KV, r *pb.RangeRequest) (*pb.RangeResponse, error) { - trace := traceutil.Get(ctx) +func Range(ctx context.Context, lg *zap.Logger, kv mvcc.KV, r *pb.RangeRequest) (resp *pb.RangeResponse, trace *traceutil.Trace, err error) { + trace = traceutil.Get(ctx) if trace.IsEmpty() { trace = traceutil.New("range", lg) ctx = context.WithValue(ctx, traceutil.TraceKey, trace) } txnRead := kv.Read(mvcc.ConcurrentReadTxMode, trace) defer txnRead.End() - return executeRange(ctx, lg, txnRead, r) + resp, err = executeRange(ctx, lg, txnRead, r) + return resp, trace, err } func executeRange(ctx context.Context, lg *zap.Logger, txnRead mvcc.TxnRead, r *pb.RangeRequest) (*pb.RangeResponse, error) { @@ -380,7 +391,7 @@ func executeTxn(ctx context.Context, lg *zap.Logger, txnWrite mvcc.TxnWrite, rt respi.(*pb.ResponseOp_ResponsePut).ResponsePut = resp trace.StopSubTrace() case *pb.RequestOp_RequestDeleteRange: - resp, err := deleteRange(txnWrite, tv.RequestDeleteRange) + resp, err := deleteRange(ctx, txnWrite, tv.RequestDeleteRange) if err != nil { return 0, fmt.Errorf("applyTxn: failed DeleteRange: %w", err) } diff --git a/server/etcdserver/v3_server.go b/server/etcdserver/v3_server.go index c25108aaa..a71a633fa 100644 --- a/server/etcdserver/v3_server.go +++ b/server/etcdserver/v3_server.go @@ -131,7 +131,7 @@ func (s *EtcdServer) Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeRe return s.authStore.IsRangePermitted(ai, r.Key, r.RangeEnd) } - get := func() { resp, err = txn.Range(ctx, s.Logger(), s.KV(), r) } + get := func() { resp, _, err = txn.Range(ctx, s.Logger(), s.KV(), r) } if serr := s.doSerialize(ctx, chk, get); serr != nil { err = serr return nil, err