EZP-21412: Improvements in DFS under concurrency load

Following the additions to dfs/oracle in ezsystems/ezoracle#15, eZDFSFileHandler was made a bit smarter when storing cache. It will abort generation if an error occured, and ignore the case where cache couldn't be stored for a known reason.
ezsystems · Sep 5, 2013 · 08ee40a · 08ee40a
1 parent 64f590e
commit 08ee40a
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 24 deletions.
diff --git a/kernel/private/classes/clusterfilehandlers/dfsbackends/mysqlbackenderror.php b/kernel/private/classes/clusterfilehandlers/dfsbackends/mysqlbackenderror.php
@@ -20,6 +20,7 @@ function eZDFSMySQLBackendError( $value, $text )
         $this->errorValue = $value;
         $this->errorText  = $text;
     }
-}
 
-?>
+    public $errorValue;
+    public $errorText;
+}
diff --git a/kernel/private/classes/clusterfilehandlers/dfsbackends/mysqli.php b/kernel/private/classes/clusterfilehandlers/dfsbackends/mysqli.php
@@ -843,10 +843,12 @@ function _store( $filePath, $datatype, $scope, $fname = false )
         else
             $fname = "_store($filePath, $datatype, $scope)";
 
-        $this->_protect( array( $this, '_storeInner' ), $fname,
+        $return = $this->_protect( array( $this, '_storeInner' ), $fname,
                          $filePath, $datatype, $scope, $fname );
 
         $this->eventHandler->notify( 'cluster/deleteFile', array( $filePath ) );
+
+        return $return;
     }
 
     /**
@@ -882,7 +884,7 @@ function _storeInner( $filePath, $datatype, $scope, $fname )
             "datatype=VALUES(datatype), scope=VALUES(scope), size=VALUES(size), mtime=VALUES(mtime), expired=VALUES(expired)",
             $fname ) === false )
         {
-            return $this->_fail( "Failed to insert file metadata while storing. Possible race condition" );
+            return false;
         }
 
         // copy given $filePath to DFS
@@ -912,8 +914,16 @@ function _storeContents( $filePath, $contents, $scope, $datatype, $mtime = false
         else
             $fname = "_storeContents($filePath, ..., $scope, $datatype)";
 
-        $this->_protect( array( $this, '_storeContentsInner' ), $fname,
-                         $filePath, $contents, $scope, $datatype, $mtime, $fname );
+        return $this->_protect(
+            array( $this, '_storeContentsInner' ),
+            $fname,
+            $filePath,
+            $contents,
+            $scope,
+            $datatype,
+            $mtime,
+            $fname
+        );
     }
 
     function _storeContentsInner( $filePath, $contents, $scope, $datatype, $curTime, $fname )
@@ -937,7 +947,7 @@ function _storeContentsInner( $filePath, $contents, $scope, $datatype, $curTime,
             "datatype=VALUES(datatype), name_trunk='$nameTrunk', scope=VALUES(scope), size=VALUES(size), mtime=VALUES(mtime), expired=VALUES(expired)",
             $fname ) === false )
         {
-            return $this->_fail( "Failed to insert file metadata while storing contents. Possible race condition" );
+            return false;
         }
 
         if ( !$this->dfsbackend->createFileOnDFS( $filePath, $contents ) )
@@ -1063,7 +1073,7 @@ protected function _insertUpdate( $table, $array, $update, $fname, $reportError
         if ( !$res )
         {
             // @todo Throw an exception
-            return false;
+            return $this->_fail( $query, __METHOD__ );
         }
         return mysqli_insert_id( $this->db );
     }

diff --git a/kernel/private/classes/clusterfilehandlers/ezdfsfilehandler.php b/kernel/private/classes/clusterfilehandlers/ezdfsfilehandler.php
@@ -237,12 +237,13 @@ public function storeContents( $contents, $scope = false, $datatype = false, $st
         eZDebugSetting::writeDebug( 'kernel-clustering', "dfs::storeContents( '$filePath' )" );
 
         // the file is stored with the current time as mtime
-        self::$dbbackend->_storeContents( $filePath, $contents, $scope, $datatype );
-
-        if ( $storeLocally )
+        $result = self::$dbbackend->_storeContents( $filePath, $contents, $scope, $datatype );
+        if ( $result && $storeLocally )
         {
             eZFile::create( basename( $filePath ), dirname( $filePath ), $contents, true );
         }
+
+        return $result;
     }
 
     /**
@@ -860,22 +861,20 @@ public function storeCache( $fileData )
             return $result;
         }
 
-        // the .generating file is stored to DFS. $storeLocally is set to false
-        // since we don't want to store the .generating file locally, only
-        // the final file.
-        $this->storeContents( $binaryData, $scope, $datatype, $storeLocally = false );
-
-        // we end the cache generation process, so that the .generating file
-        // is removed (we don't need to rename since contents was already stored
-        // above, using fileStoreContents
-        $this->endCacheGeneration();
+        // Distinguish bool from eZClusterFileFailure, and call abortCacheGeneration()
+        $storeContentsResult = $this->storeContents( $binaryData, $scope, $datatype, $storeLocally = false );
 
-        if ( self::LOCAL_CACHE )
+        // Cache was stored, we end cache generation
+        if ( $storeContentsResult === true )
         {
-            eZDebugSetting::writeDebug( 'kernel-clustering',
-                "Creating local copy of the file", "dfs::storeCache( '{$this->filePath}' )" );
-            eZFile::create( basename( $this->filePath ), dirname( $this->filePath ), $binaryData, true );
+            $this->endCacheGeneration();
+        }
+        // An unexpected error occured, we abort generation
+        else if ( $storeContentsResult instanceof eZMySQLBackendError )
+        {
+            $this->abortCacheGeneration();
         }
+        // We don't do anything if false (not stored for known reasons) has been returned
 
         return $result;
     }