From: Mike Rylander <mrylander@gmail.com>
Date: Mon, 2 Nov 2020 15:25:58 +0000 (-0500)
Subject: LP#1902583: Retry cache commands exactly one time
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=d5b95d4184f70ecfc46b83fee8307874ac9bd681;p=working%2FOpenSRF.git

LP#1902583: Retry cache commands exactly one time

If OpenSRF apps want any chance to recover from a memcached server
failure they need to retry the command at least once, because
libmemcached does not (nor do memcached proxies).  This commit adds that
single retry capability.

Signed-off-by: Mike Rylander <mrylander@gmail.com>
---

diff --git a/src/libopensrf/osrf_cache.c b/src/libopensrf/osrf_cache.c
index b6e9f1b..d99fce7 100644
--- a/src/libopensrf/osrf_cache.c
+++ b/src/libopensrf/osrf_cache.c
@@ -84,8 +84,12 @@ int osrfCachePutString( const char* key, const char* value, time_t seconds ) {
 	/* add or overwrite existing key:value pair */
 	rc = memcached_set(_osrfCache, clean_key, strlen(clean_key), value, strlen(value), seconds, 0);
 	if (rc != MEMCACHED_SUCCESS) {
-		osrfLogError(OSRF_LOG_MARK, "Failed to cache key:value [%s]:[%s] - %s",
-			key, value, memcached_strerror(_osrfCache, rc));
+		// Try one more time, maybe a server failure
+		rc = memcached_set(_osrfCache, clean_key, strlen(clean_key), value, strlen(value), seconds, 0);
+		if (rc != MEMCACHED_SUCCESS) {
+			osrfLogError(OSRF_LOG_MARK, "Failed to cache key:value [%s]:[%s] - %s",
+				key, value, memcached_strerror(_osrfCache, rc));
+		}
 	}
 
 	free(clean_key);
@@ -100,11 +104,15 @@ jsonObject* osrfCacheGetObject( const char* key, ... ) {
 	if( key ) {
 		char* clean_key = _clean_key( key );
 		const char* data = (const char*) memcached_get(_osrfCache, clean_key, strlen(clean_key), &val_len, &flags, &rc);
-		free(clean_key);
 		if (rc != MEMCACHED_SUCCESS) {
-			osrfLogDebug(OSRF_LOG_MARK, "Failed to get key [%s] - %s",
-				key, memcached_strerror(_osrfCache, rc));
+			// Try one more time, maybe a server failure
+			data = (const char*) memcached_get(_osrfCache, clean_key, strlen(clean_key), &val_len, &flags, &rc);
+			if (rc != MEMCACHED_SUCCESS) {
+				osrfLogDebug(OSRF_LOG_MARK, "Failed to get key [%s] - %s",
+					key, memcached_strerror(_osrfCache, rc));
+			}
 		}
+		free(clean_key);
 		if( data ) {
 			osrfLogInternal( OSRF_LOG_MARK, "osrfCacheGetObject(): Returning object (key=%s): %s", key, data);
 			obj = jsonParse( data );
@@ -122,11 +130,15 @@ char* osrfCacheGetString( const char* key, ... ) {
 	if( key ) {
 		char* clean_key = _clean_key( key );
 		char* data = (char*) memcached_get(_osrfCache, clean_key, strlen(clean_key), &val_len, &flags, &rc);
-		free(clean_key);
 		if (rc != MEMCACHED_SUCCESS) {
-			osrfLogDebug(OSRF_LOG_MARK, "Failed to get key [%s] - %s",
-				key, memcached_strerror(_osrfCache, rc));
+			// Try one more time, maybe a server failure
+			data = (char*) memcached_get(_osrfCache, clean_key, strlen(clean_key), &val_len, &flags, &rc);
+			if (rc != MEMCACHED_SUCCESS) {
+				osrfLogDebug(OSRF_LOG_MARK, "Failed to get key [%s] - %s",
+					key, memcached_strerror(_osrfCache, rc));
+			}
 		}
+		free(clean_key);
 		osrfLogInternal( OSRF_LOG_MARK, "osrfCacheGetString(): Returning object (key=%s): %s", key, data);
 		if(!data) osrfLogDebug(OSRF_LOG_MARK, "No cache data exists with key %s", key);
 		return data;
@@ -140,11 +152,15 @@ int osrfCacheRemove( const char* key, ... ) {
 	if( key ) {
 		char* clean_key = _clean_key( key );
 		rc = memcached_delete(_osrfCache, clean_key, strlen(clean_key), 0 );
-		free(clean_key);
 		if (rc != MEMCACHED_SUCCESS && rc != MEMCACHED_BUFFERED) {
-			osrfLogDebug(OSRF_LOG_MARK, "Failed to delete key [%s] - %s",
-				key, memcached_strerror(_osrfCache, rc));
+			// Try one more time, maybe a server failure
+			rc = memcached_delete(_osrfCache, clean_key, strlen(clean_key), 0 );
+			if (rc != MEMCACHED_SUCCESS && rc != MEMCACHED_BUFFERED) {
+				osrfLogDebug(OSRF_LOG_MARK, "Failed to delete key [%s] - %s",
+					key, memcached_strerror(_osrfCache, rc));
+			}
 		}
+		free(clean_key);
 		return 0;
 	}
 	return -1;
diff --git a/src/perl/lib/OpenSRF/Utils/Cache.pm b/src/perl/lib/OpenSRF/Utils/Cache.pm
index 36721d9..1ceae48 100644
--- a/src/perl/lib/OpenSRF/Utils/Cache.pm
+++ b/src/perl/lib/OpenSRF/Utils/Cache.pm
@@ -124,8 +124,10 @@ sub put_cache {
 	$expiretime ||= $max_persist_time;
 
 	unless( $self->{memcache}->set( $key, $value, $expiretime ) ) {
-		$log->error("Unable to store $key => [".length($value)." bytes]  in memcached server" );
-		return undef;
+		unless( $self->{memcache}->set( $key, $value, $expiretime ) ) { # retry exactly once on possible server failure
+			$log->error("Unable to store $key => [".length($value)." bytes]  in memcached server" );
+			return undef;
+		}
 	}
 
 	$log->debug("Stored $key => $value in memcached server", INTERNAL);
@@ -165,7 +167,8 @@ sub delete_cache {
 	$key = _clean_cache_key($key);
 	return undef if $key eq ''; # no zero-length keys
 	if($self->{persist}){ _load_methods(); }
-	$self->{memcache}->delete($key);
+	# This will retry exactly once on failed delete, which can happen b/c of a server failure
+	$self->{memcache}->delete($key) || $self->{memcache}->delete($key);
 	if( $self->{persist} ) {
 		$persist_destroy_slot->run("_CACHEVAL_$key");
 	}
@@ -186,7 +189,8 @@ sub get_cache {
 
 	return undef if $key eq ''; # no zero-length keys
 
-	my $val = $self->{memcache}->get( $key );
+	# This will retry exactly once on an undef response, which can happen b/c of a server failure
+	my $val = $self->{memcache}->get( $key ) // $self->{memcache}->get( $key );
 	return OpenSRF::Utils::JSON->JSON2perl($val) if defined($val);
 
 	if($self->{persist}){ _load_methods(); }