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 (file)
--- 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 (file)
--- 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(); }