Skip to content

Commit

Permalink
Revert stickiness (grpc#2175)
Browse files Browse the repository at this point in the history 
Stickiness will be reimplemented as part of a balancer/resolver redesigning/extending.
  • Loading branch information
@menghanl
menghanl authored Jun 26, 2018
1 parent 4f70de2 commit b39aa9e
Show file tree
Hide file tree
Showing 8 changed files with 3 additions and 839 deletions.
46 changes: 0 additions & 46 deletions Documentation/stickiness.md
View file

This file was deleted.

21 changes: 0 additions & 21 deletions clientconn.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,6 @@ func (cc *ClientConn) switchBalancer(name string) {
if cc.balancerWrapper != nil {
cc.balancerWrapper.close()
}
// Clear all stickiness state.
cc.blockingpicker.clearStickinessState()

builder := balancer.Get(name)
if builder == nil {
Expand Down Expand Up @@ -1046,18 +1044,6 @@ func (cc *ClientConn) handleServiceConfig(js string) error {
cc.balancerWrapper.handleResolvedAddrs(cc.curAddresses, nil)
}
}

if envConfigStickinessOn {
var newStickinessMDKey string
if sc.stickinessMetadataKey != nil && *sc.stickinessMetadataKey != "" {
newStickinessMDKey = *sc.stickinessMetadataKey
}
// newStickinessMDKey is "" if one of the following happens:
// - stickinessMetadataKey is set to ""
// - stickinessMetadataKey field doesn't exist in service config
cc.blockingpicker.updateStickinessMDKey(strings.ToLower(newStickinessMDKey))
}

cc.mu.Unlock()
return nil
}
Expand Down Expand Up @@ -1551,13 +1537,6 @@ func (ac *addrConn) getState() connectivity.State {
return ac.state
}

func (ac *addrConn) getCurAddr() (ret resolver.Address) {
ac.mu.Lock()
ret = ac.curAddr
ac.mu.Unlock()
return
}

func (ac *addrConn) ChannelzMetric() *channelz.ChannelInternalMetric {
ac.mu.Lock()
addr := ac.curAddr.Addr
Expand Down
37 changes: 0 additions & 37 deletions envconfig.go
View file

This file was deleted.

158 changes: 1 addition & 157 deletions picker_wrapper.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,12 @@ package grpc
import (
"io"
"sync"
"sync/atomic"

"golang.org/x/net/context"
"google.golang.org/grpc/balancer"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/grpclog"
"google.golang.org/grpc/internal/channelz"
"google.golang.org/grpc/metadata"
"google.golang.org/grpc/resolver"
"google.golang.org/grpc/status"
"google.golang.org/grpc/transport"
)
Expand All @@ -45,16 +42,10 @@ type pickerWrapper struct {
// The latest connection happened.
connErrMu sync.Mutex
connErr error

stickinessMDKey atomic.Value
stickiness *stickyStore
}

func newPickerWrapper() *pickerWrapper {
bp := &pickerWrapper{
blockingCh: make(chan struct{}),
stickiness: newStickyStore(),
}
bp := &pickerWrapper{blockingCh: make(chan struct{})}
return bp
}

Expand All @@ -71,27 +62,6 @@ func (bp *pickerWrapper) connectionError() error {
return err
}

func (bp *pickerWrapper) updateStickinessMDKey(newKey string) {
// No need to check ok because mdKey == "" if ok == false.
if oldKey, _ := bp.stickinessMDKey.Load().(string); oldKey != newKey {
bp.stickinessMDKey.Store(newKey)
bp.stickiness.reset(newKey)
}
}

func (bp *pickerWrapper) getStickinessMDKey() string {
// No need to check ok because mdKey == "" if ok == false.
mdKey, _ := bp.stickinessMDKey.Load().(string)
return mdKey
}

func (bp *pickerWrapper) clearStickinessState() {
if oldKey := bp.getStickinessMDKey(); oldKey != "" {
// There's no need to reset store if mdKey was "".
bp.stickiness.reset(oldKey)
}
}

// updatePicker is called by UpdateBalancerState. It unblocks all blocked pick.
func (bp *pickerWrapper) updatePicker(p balancer.Picker) {
bp.mu.Lock()
Expand Down Expand Up @@ -131,27 +101,6 @@ func doneChannelzWrapper(acw *acBalancerWrapper, done func(balancer.DoneInfo)) f
// - the subConn returned by the current picker is not READY
// When one of these situations happens, pick blocks until the picker gets updated.
func (bp *pickerWrapper) pick(ctx context.Context, failfast bool, opts balancer.PickOptions) (transport.ClientTransport, func(balancer.DoneInfo), error) {

mdKey := bp.getStickinessMDKey()
stickyKey, isSticky := stickyKeyFromContext(ctx, mdKey)

// Potential race here: if stickinessMDKey is updated after the above two
// lines, and this pick is a sticky pick, the following put could add an
// entry to sticky store with an outdated sticky key.
//
// The solution: keep the current md key in sticky store, and at the
// beginning of each get/put, check the mdkey against store.curMDKey.
// - Cons: one more string comparing for each get/put.
// - Pros: the string matching happens inside get/put, so the overhead for
// non-sticky RPCs will be minimal.

if isSticky {
if t, ok := bp.stickiness.get(mdKey, stickyKey); ok {
// Done function returned is always nil.
return t, nil, nil
}
}

var (
p balancer.Picker
ch chan struct{}
Expand Down Expand Up @@ -207,9 +156,6 @@ func (bp *pickerWrapper) pick(ctx context.Context, failfast bool, opts balancer.
continue
}
if t, ok := acw.getAddrConn().getReadyTransport(); ok {
if isSticky {
bp.stickiness.put(mdKey, stickyKey, acw)
}
if channelz.IsOn() {
return t, doneChannelzWrapper(acw, done), nil
}
Expand All @@ -232,105 +178,3 @@ func (bp *pickerWrapper) close() {
bp.done = true
close(bp.blockingCh)
}

const stickinessKeyCountLimit = 1000

type stickyStoreEntry struct {
acw *acBalancerWrapper
addr resolver.Address
}

type stickyStore struct {
mu sync.Mutex
// curMDKey is check before every get/put to avoid races. The operation will
// abort immediately when the given mdKey is different from the curMDKey.
curMDKey string
store *linkedMap
}

func newStickyStore() *stickyStore {
return &stickyStore{
store: newLinkedMap(),
}
}

// reset clears the map in stickyStore, and set the currentMDKey to newMDKey.
func (ss *stickyStore) reset(newMDKey string) {
ss.mu.Lock()
ss.curMDKey = newMDKey
ss.store.clear()
ss.mu.Unlock()
}

// stickyKey is the key to look up in store. mdKey will be checked against
// curMDKey to avoid races.
func (ss *stickyStore) put(mdKey, stickyKey string, acw *acBalancerWrapper) {
ss.mu.Lock()
defer ss.mu.Unlock()
if mdKey != ss.curMDKey {
return
}
// TODO(stickiness): limit the total number of entries.
ss.store.put(stickyKey, &stickyStoreEntry{
acw: acw,
addr: acw.getAddrConn().getCurAddr(),
})
if ss.store.len() > stickinessKeyCountLimit {
ss.store.removeOldest()
}
}

// stickyKey is the key to look up in store. mdKey will be checked against
// curMDKey to avoid races.
func (ss *stickyStore) get(mdKey, stickyKey string) (transport.ClientTransport, bool) {
ss.mu.Lock()
defer ss.mu.Unlock()
if mdKey != ss.curMDKey {
return nil, false
}
entry, ok := ss.store.get(stickyKey)
if !ok {
return nil, false
}
ac := entry.acw.getAddrConn()
if ac.getCurAddr() != entry.addr {
ss.store.remove(stickyKey)
return nil, false
}
t, ok := ac.getReadyTransport()
if !ok {
ss.store.remove(stickyKey)
return nil, false
}
return t, true
}

// Get one value from metadata in ctx with key stickinessMDKey.
//
// It returns "", false if stickinessMDKey is an empty string.
func stickyKeyFromContext(ctx context.Context, stickinessMDKey string) (string, bool) {
if stickinessMDKey == "" {
return "", false
}

md, added, ok := metadata.FromOutgoingContextRaw(ctx)
if !ok {
return "", false
}

if vv, ok := md[stickinessMDKey]; ok {
if len(vv) > 0 {
return vv[0], true
}
}

for _, ss := range added {
for i := 0; i < len(ss)-1; i += 2 {
if ss[i] == stickinessMDKey {
return ss[i+1], true
}
}
}

return "", false
}
9 changes: 2 additions & 7 deletions service_config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@ type ServiceConfig struct {
// If there's no exact match, look for the default config for the service (/service/) and use the corresponding MethodConfig if it exists.
// Otherwise, the method has no MethodConfig to use.
Methods map[string]MethodConfig

stickinessMetadataKey *string
}

func parseDuration(s *string) (*time.Duration, error) {
Expand Down Expand Up @@ -148,9 +146,8 @@ type jsonMC struct {

// TODO(lyuxuan): delete this struct after cleaning up old service config implementation.
type jsonSC struct {
LoadBalancingPolicy *string
StickinessMetadataKey *string
MethodConfig *[]jsonMC
LoadBalancingPolicy *string
MethodConfig *[]jsonMC
}

func parseServiceConfig(js string) (ServiceConfig, error) {
Expand All @@ -163,8 +160,6 @@ func parseServiceConfig(js string) (ServiceConfig, error) {
sc := ServiceConfig{
LB: rsc.LoadBalancingPolicy,
Methods: make(map[string]MethodConfig),

stickinessMetadataKey: rsc.StickinessMetadataKey,
}
if rsc.MethodConfig == nil {
return sc, nil
Expand Down
Loading

0 comments on commit b39aa9e

Please sign in to comment.