Add new metric And logger possiblity for peers error

[Tommy-42]

Introducing a new metric GetFromPeersSlowestDuration which will be recording the slowest
call made to getFromPeers.

This new metric requires to add a method Store to override the atomic value instead of just increasing its value.

Along with this new metric, I Introduce SetLogger() to set a logger and allow getFromPeers errors to be logged ( previously fully ignored ).

Also fixing a data race that could happen upon deletion of a key from the cache

[thrawn01]

Would be nice to include the peer such that log message looks like

error retrieving key from peer 'scout-n01'

Also, instead of prefix message with groupcache: we add a category field. "category": "groupcache" which makes it easy to search ELK stack for groupcache related errors.

[Tommy-42]

Regarding :

Would be nice to include the peer such that log message looks like
  > error retrieving key from peer 'scout-n01'

Do you want :

• the peer's hostname that make the call to the remote peers

Or :

• the remote peer’s hostname that generate the error when being called

So,
If it is the peer's hostname that make the call to the remote peers
when setting the logger with SetLogger(), the logger can be setup with a default field like this :

logger = logger.WithFields(logrus.Fields{
	"peer": hostname,
})
groupcache.SetLogger(logger)

if it is the remote peer’s hostname that generate the error when being called
I don't see how to do it without modifying the code a lot ?

[thrawn01]

ELK stacks should take care of identifying where the log entry is coming from. error retrieving key from peer 'scout-n01' is sufficient.

[thrawn01]

The peer variable should have the hostname/ip of the remote peer.

[Tommy-42]

peer is a variable of type ProtoGetter which is an interface

type ProtoGetter interface {
	Get(context context.Context, in *pb.GetRequest, out *pb.GetResponse) error
	Remove(context context.Context, in *pb.GetRequest) error
}

So we don't have access to any private field that might contains the return var, Am I wrong, or miss something ?

[thrawn01]

The implementation is httpGetter, you can add a new method GetURL() to the ProtoGetter interface which will return the peer url. This interface is only used internal to groupcache for contacting peers. Does that make sense?

[Tommy-42]

ok gotcha,

error now will looks like :

ERRO[0035] error retrieving key from peer 'http://MBP-tommy-42.local:5071'  category=groupcache err="dial tcp 127.0.0.1:5071: connect: connection refused" key="blabla" peer=MBP-tommy-42.local

[thrawn01]

You can remove this now; I think?

[Tommy-42]

hmm nop :D , fakePeers Implement the ProtoGetter interface with the new GetURL() method that need to be implemented

[thrawn01]

GetURL(key string) string is a different interface than what ProtoGetter requires GetURL() string

[thrawn01]

also, fakePeers is a collection of []ProtoGetter not an implementation of the ProtoGetter interface, as such it doesn't have to implement the ProtoGetter interface, right?

[Tommy-42]

ah ok, miss-understand here :D , I removed it from fakePeerS**** but added it on fakePeer so we should be good now :D

[thrawn01]

Awesome work, thanks!