Skip to content

Commit 95b0bb1

Browse files
smukiltharanga
authored andcommitted
Add a metric to track "Wrong key returned" errors
This change adds "wrongKeyReturned" as a metric and increments it everytime we see it in action. This change was added as we noticed some of these error messages in the logs, and we'd like to track the frequence of these errors. Note that even prior to this change, we did not return the wrong key back to the calling application but instead count it as a miss. Print stack trace when we notice a Wrong Key error Consolidate duplicate code under checkWrongKeyReturned() Address review comments for wrong key returned metric - Better logging to catch if returned key is from a different host - Make sure stack trace goes to logs instead of stdout - Naming nits addressed
1 parent 64da6ff commit 95b0bb1

File tree

3 files changed

+33
-13
lines changed

3 files changed

+33
-13
lines changed

evcache-core/src/main/java/com/netflix/evcache/metrics/EVCacheMetricsFactory.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,7 @@ public String getStatusCode(StatusCode sc) {
390390
public static final String CHUNK_DATA_SIZE = "dataSize";
391391
public static final String NOT_AVAILABLE = "notAvailable";
392392
public static final String NOT_ACTIVE = "notActive";
393+
public static final String WRONG_KEY_RETURNED = "wrongKeyReturned";
393394

394395
public static final String INITIAL = "initial";
395396
public static final String SECOND = "second";

evcache-core/src/main/java/com/netflix/evcache/pool/EVCacheClient.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,9 @@ private void incrementFailure(String metric, EVCache.Call call, String host) {
259259
counter.increment();
260260
}
261261

262+
public void reportWrongKeyReturned(String hostName) {
263+
incrementFailure(EVCacheMetricsFactory.WRONG_KEY_RETURNED, null, hostName);
264+
}
262265

263266
private boolean ensureWriteQueueSize(MemcachedNode node, String key, EVCache.Call call) throws EVCacheException {
264267
if (node instanceof EVCacheNode) {

evcache-core/src/main/java/net/spy/memcached/EVCacheMemcachedClient.java

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,22 @@ public <T> GetFuture<T> asyncGet(final String key, final Transcoder<T> tc) {
100100
throw new UnsupportedOperationException("asyncGet");
101101
}
102102

103+
// Returns 'true' if keys don't match and logs & reports the error.
104+
// Returns 'false' if keys match.
105+
// TODO: Consider removing this code once we've fixed the Wrong key bug(s)
106+
private boolean isWrongKeyReturned(String original_key, String returned_key) {
107+
if (!original_key.equals(returned_key)) {
108+
// If they keys don't match, log the error along with the key owning host's information and stack trace.
109+
final String original_host = getHostNameByKey(original_key);
110+
final String returned_host = getHostNameByKey(returned_key);
111+
log.error("Wrong key returned. Key - " + original_key + " (Host: " + original_host + ") ; Returned Key "
112+
+ returned_key + " (Host: " + returned_host + ")", new Exception());
113+
client.reportWrongKeyReturned(original_host);
114+
return true;
115+
}
116+
return false;
117+
}
118+
103119
public <T> EVCacheOperationFuture<T> asyncGet(final String key, final Transcoder<T> tc, EVCacheGetOperationListener<T> listener) {
104120
final CountDownLatch latch = new CountDownLatch(1);
105121
final EVCacheOperationFuture<T> rv = new EVCacheOperationFuture<T>(key, latch, new AtomicReference<T>(null), readTimeout.get().intValue(), executorService, client);
@@ -126,10 +142,8 @@ public void receivedStatus(OperationStatus status) {
126142
@SuppressWarnings("unchecked")
127143
public void gotData(String k, int flags, byte[] data) {
128144

129-
if (!key.equals(k)) {
130-
log.error("Wrong key returned. Key - " + key + "; Returned Key " + k);
131-
return;
132-
}
145+
if (isWrongKeyReturned(key, k)) return;
146+
133147
if (log.isDebugEnabled() && client.getPool().getEVCacheClientPoolManager().shouldLog(appName)) log.debug("Read data : key " + key + "; flags : " + flags + "; data : " + data);
134148
if (data != null) {
135149
if (log.isDebugEnabled() && client.getPool().getEVCacheClientPoolManager().shouldLog(appName)) log.debug("Key : " + key + "; val size : " + data.length);
@@ -255,7 +269,8 @@ public void complete() {
255269
}
256270

257271
public void gotData(String k, int flags, long cas, byte[] data) {
258-
if (!key.equals(k)) log.warn("Wrong key returned. Key - " + key + "; Returned Key " + k);
272+
if (isWrongKeyReturned(key, k)) return;
273+
259274
if (data != null) getDataSizeDistributionSummary(EVCacheMetricsFactory.GET_AND_TOUCH_OPERATION, EVCacheMetricsFactory.READ, EVCacheMetricsFactory.IPC_SIZE_INBOUND).record(data.length);
260275
val = new CASValue<T>(cas, tc.decode(new CachedData(flags, data, tc.getMaxSize())));
261276
}
@@ -602,6 +617,11 @@ public int getReadMetricMaxValue() {
602617
return maxReadDuration.get().intValue();
603618
}
604619

620+
private String getHostNameByKey(String key) {
621+
MemcachedNode evcNode = getEVCacheNode(key);
622+
return getHostName(evcNode.getSocketAddress());
623+
}
624+
605625
private String getHostName(SocketAddress sa) {
606626
if (sa == null) return null;
607627
if(sa instanceof InetSocketAddress) {
@@ -722,10 +742,8 @@ public void receivedStatus(OperationStatus status) {
722742
@Override
723743
public void gotMetaData(String k, char flag, String fVal) {
724744
if (log.isDebugEnabled()) log.debug("key " + k + "; val : " + fVal + "; flag : " + flag);
725-
if (!key.equals(k)) {
726-
log.error("Wrong key returned. Expected Key - " + key + "; Returned Key " + k);
727-
return;
728-
}
745+
if (isWrongKeyReturned(key, k)) return;
746+
729747
switch (flag) {
730748
case 's':
731749
evItem.getItemMetaData().setSizeInBytes(Integer.parseInt(fVal));
@@ -765,10 +783,8 @@ public void gotMetaData(String k, char flag, String fVal) {
765783
@Override
766784
public void gotData(String k, int flag, byte[] data) {
767785
if (log.isDebugEnabled() && client.getPool().getEVCacheClientPoolManager().shouldLog(appName)) log.debug("Read data : key " + k + "; flags : " + flag + "; data : " + data);
768-
if (!key.equals(k)) {
769-
log.error("Wrong key returned. Expected Key - " + key + "; Returned Key " + k);
770-
return;
771-
}
786+
if (isWrongKeyReturned(key, k)) return;
787+
772788
if (data != null) {
773789
if (log.isDebugEnabled() && client.getPool().getEVCacheClientPoolManager().shouldLog(appName)) log.debug("Key : " + k + "; val size : " + data.length);
774790
getDataSizeDistributionSummary(EVCacheMetricsFactory.META_GET_OPERATION, EVCacheMetricsFactory.READ, EVCacheMetricsFactory.IPC_SIZE_INBOUND).record(data.length);

0 commit comments

Comments
 (0)