Skip to content

Commit

Permalink
Remove shouldApplyV3 from the v3 applier
Browse files Browse the repository at this point in the history 
Signed-off-by: Marek Siarkowicz <[email protected]>
  • Loading branch information
@serathius
serathius committed Nov 24, 2023
1 parent d22c00c commit 2ad2155
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 28 deletions.
8 changes: 4 additions & 4 deletions server/etcdserver/apply/apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,13 @@ type Result struct {
Trace *traceutil.Trace
}

type applyFunc func(ctx context.Context, r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *Result
type applyFunc func(ctx context.Context, r *pb.InternalRaftRequest) *Result

// applierV3 is the interface for processing V3 raft messages
type applierV3 interface {
// Apply executes the generic portion of application logic for the current applier, but
// delegates the actual execution to the applyFunc method.
Apply(ctx context.Context, r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3, applyFunc applyFunc) *Result
Apply(ctx context.Context, r *pb.InternalRaftRequest, applyFunc applyFunc) *Result

Put(ctx context.Context, p *pb.PutRequest) (*pb.PutResponse, *traceutil.Trace, error)
Range(ctx context.Context, r *pb.RangeRequest) (*pb.RangeResponse, *traceutil.Trace, error)
Expand Down Expand Up @@ -146,8 +146,8 @@ func newApplierV3Backend(
txnModeWriteWithSharedBuffer: txnModeWriteWithSharedBuffer}
}

func (a *applierV3backend) Apply(ctx context.Context, r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3, applyFunc applyFunc) *Result {
return applyFunc(ctx, r, shouldApplyV3)
func (a *applierV3backend) Apply(ctx context.Context, r *pb.InternalRaftRequest, applyFunc applyFunc) *Result {
return applyFunc(ctx, r)
}

func (a *applierV3backend) Put(ctx context.Context, p *pb.PutRequest) (resp *pb.PutResponse, trace *traceutil.Trace, err error) {
Expand Down
5 changes: 2 additions & 3 deletions server/etcdserver/apply/apply_auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
pb "go.etcd.io/etcd/api/v3/etcdserverpb"
"go.etcd.io/etcd/pkg/v3/traceutil"
"go.etcd.io/etcd/server/v3/auth"
"go.etcd.io/etcd/server/v3/etcdserver/api/membership"
"go.etcd.io/etcd/server/v3/etcdserver/txn"
"go.etcd.io/etcd/server/v3/lease"
)
Expand All @@ -42,7 +41,7 @@ func newAuthApplierV3(as auth.AuthStore, base applierV3, lessor lease.Lessor) *a
return &authApplierV3{applierV3: base, as: as, lessor: lessor}
}

func (aa *authApplierV3) Apply(ctx context.Context, r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3, applyFunc applyFunc) *Result {
func (aa *authApplierV3) Apply(ctx context.Context, r *pb.InternalRaftRequest, applyFunc applyFunc) *Result {
aa.mu.Lock()
defer aa.mu.Unlock()
if r.Header != nil {
Expand All @@ -58,7 +57,7 @@ func (aa *authApplierV3) Apply(ctx context.Context, r *pb.InternalRaftRequest, s
return &Result{Err: err}
}
}
ret := aa.applierV3.Apply(ctx, r, shouldApplyV3, applyFunc)
ret := aa.applierV3.Apply(ctx, r, applyFunc)
aa.authInfo.Username = ""
aa.authInfo.Revision = 0
return ret
Expand Down
6 changes: 3 additions & 3 deletions server/etcdserver/apply/apply_auth_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func dummyIndexWaiter(_ uint64) <-chan struct{} {
return ch
}

func dummyApplyFunc(_ context.Context, _ *pb.InternalRaftRequest, _ membership.ShouldApplyV3) *Result {
func dummyApplyFunc(_ context.Context, _ *pb.InternalRaftRequest) *Result {
return &Result{}
}

Expand Down Expand Up @@ -215,7 +215,7 @@ func TestAuthApplierV3_Apply(t *testing.T) {
defer cancel()
for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
result := authApplier.Apply(ctx, tc.request, false, dummyApplyFunc)
result := authApplier.Apply(ctx, tc.request, dummyApplyFunc)
require.Equalf(t, result, tc.expectResult, "Apply: got %v, expect: %v", result, tc.expectResult)
})
}
Expand Down Expand Up @@ -386,7 +386,7 @@ func TestAuthApplierV3_AdminPermission(t *testing.T) {
if tc.adminPermissionNeeded {
tc.request.Header = &pb.RequestHeader{Username: userReadOnly}
}
result := authApplier.Apply(ctx, tc.request, false, dummyApplyFunc)
result := authApplier.Apply(ctx, tc.request, dummyApplyFunc)
require.Equal(t, result.Err == auth.ErrPermissionDenied, tc.adminPermissionNeeded,
"Admin permission needed: got %v, expect: %v", result.Err == auth.ErrPermissionDenied, tc.adminPermissionNeeded)
})
Expand Down
12 changes: 4 additions & 8 deletions server/etcdserver/apply/uber_applier.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
)

type UberApplier interface {
Apply(r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *Result
Apply(r *pb.InternalRaftRequest) *Result
}

type uberApplier struct {
Expand Down Expand Up @@ -108,18 +108,18 @@ func (a *uberApplier) restoreAlarms() {
}
}

func (a *uberApplier) Apply(r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *Result {
func (a *uberApplier) Apply(r *pb.InternalRaftRequest) *Result {
// We first execute chain of Apply() calls down the hierarchy:
// (i.e. CorruptApplier -> CappedApplier -> Auth -> Quota -> Backend),
// then dispatch() unpacks the request to a specific method (like Put),
// that gets executed down the hierarchy again:
// i.e. CorruptApplier.Put(CappedApplier.Put(...(BackendApplier.Put(...)))).
return a.applyV3.Apply(context.TODO(), r, shouldApplyV3, a.dispatch)
return a.applyV3.Apply(context.TODO(), r, a.dispatch)
}

// dispatch translates the request (r) into appropriate call (like Put) on
// the underlying applyV3 object.
func (a *uberApplier) dispatch(ctx context.Context, r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *Result {
func (a *uberApplier) dispatch(ctx context.Context, r *pb.InternalRaftRequest) *Result {
op := "unknown"
ar := &Result{}
defer func(start time.Time) {
Expand All @@ -131,10 +131,6 @@ func (a *uberApplier) dispatch(ctx context.Context, r *pb.InternalRaftRequest, s
}
}(time.Now())

if !shouldApplyV3 {
return nil
}

switch {
case r.Range != nil:
op = "Range"
Expand Down
16 changes: 8 additions & 8 deletions server/etcdserver/apply/uber_applier_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,13 @@ func TestUberApplier_Alarm_Corrupt(t *testing.T) {
MemberID: memberId,
Alarm: pb.AlarmType_CORRUPT,
},
}, true)
})
require.NotNil(t, result)
require.Nil(t, result.Err)

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
result = ua.Apply(tc.request, true)
result = ua.Apply(tc.request)
require.NotNil(t, result)
require.Equalf(t, tc.expectError, result.Err, "Apply: got %v, expect: %v", result.Err, tc.expectError)
})
Expand Down Expand Up @@ -227,13 +227,13 @@ func TestUberApplier_Alarm_Quota(t *testing.T) {
MemberID: memberId,
Alarm: pb.AlarmType_NOSPACE,
},
}, true)
})
require.NotNil(t, result)
require.Nil(t, result.Err)

for _, tc := range tcs {
t.Run(tc.name, func(t *testing.T) {
result = ua.Apply(tc.request, true)
result = ua.Apply(tc.request)
require.NotNil(t, result)
require.Equalf(t, tc.expectError, result.Err, "Apply: got %v, expect: %v", result.Err, tc.expectError)
})
Expand All @@ -250,11 +250,11 @@ func TestUberApplier_Alarm_Deactivate(t *testing.T) {
MemberID: memberId,
Alarm: pb.AlarmType_NOSPACE,
},
}, true)
})
require.NotNil(t, result)
require.Nil(t, result.Err)

result = ua.Apply(&pb.InternalRaftRequest{Put: &pb.PutRequest{Key: []byte(key)}}, true)
result = ua.Apply(&pb.InternalRaftRequest{Put: &pb.PutRequest{Key: []byte(key)}})
require.NotNil(t, result)
require.Equalf(t, errors.ErrNoSpace, result.Err, "Apply: got %v, expect: %v", result.Err, errors.ErrNoSpace)

Expand All @@ -265,11 +265,11 @@ func TestUberApplier_Alarm_Deactivate(t *testing.T) {
MemberID: memberId,
Alarm: pb.AlarmType_NOSPACE,
},
}, true)
})
require.NotNil(t, result)
require.Nil(t, result.Err)

result = ua.Apply(&pb.InternalRaftRequest{Put: &pb.PutRequest{Key: []byte(key)}}, true)
result = ua.Apply(&pb.InternalRaftRequest{Put: &pb.PutRequest{Key: []byte(key)}})
require.NotNil(t, result)
require.Nil(t, result.Err)
}
5 changes: 4 additions & 1 deletion server/etcdserver/server.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1913,7 +1913,10 @@ func (s *EtcdServer) applyEntryNormal(e *raftpb.Entry, shouldApplyV3 membership.

func (s *EtcdServer) applyInternalRaftRequest(r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *apply.Result {
if r.ClusterVersionSet == nil && r.ClusterMemberAttrSet == nil && r.DowngradeInfoSet == nil {
return s.uberApply.Apply(r, shouldApplyV3)
if !shouldApplyV3 {
return nil
}
return s.uberApply.Apply(r)
}
membershipApplier := apply.NewApplierMembership(s.lg, s.cluster, s)
op := "unknown"
Expand Down
2 changes: 1 addition & 1 deletion server/etcdserver/server_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func TestApplyRepeat(t *testing.T) {

type uberApplierMock struct{}

func (uberApplierMock) Apply(r *pb.InternalRaftRequest, shouldApplyV3 membership.ShouldApplyV3) *apply2.Result {
func (uberApplierMock) Apply(r *pb.InternalRaftRequest) *apply2.Result {
return &apply2.Result{}
}

Expand Down

0 comments on commit 2ad2155

Please sign in to comment.