From 08ee40a59b2433d5979268030a201e306de1e1bb Mon Sep 17 00:00:00 2001 From: Bertrand Dunogier Date: Thu, 22 Aug 2013 19:09:25 +0200 Subject: [PATCH] EZP-21412: Improvements in DFS under concurrency load Following the additions to dfs/oracle in https://github.com/ezsystems/ezoracle/pull/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. --- .../dfsbackends/mysqlbackenderror.php | 5 +-- .../dfsbackends/mysqli.php | 22 +++++++++---- .../clusterfilehandlers/ezdfsfilehandler.php | 31 +++++++++---------- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/kernel/private/classes/clusterfilehandlers/dfsbackends/mysqlbackenderror.php b/kernel/private/classes/clusterfilehandlers/dfsbackends/mysqlbackenderror.php index 640aa5bfb1c..9f389eadbc6 100644 --- 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 index a3dd7ee9961..49b72808c60 100644 --- 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 index 0647a686191..16b12ffd1c3 100644 --- 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; }