From 6a6a0ffa24b20372b4042debbf9a17bace23f762 Mon Sep 17 00:00:00 2001 From: bruno DA SILVA Date: Mon, 27 Jan 2020 17:58:30 +0100 Subject: [PATCH] 1627 - Ticket ref sometimes duplicate :bug: fix ref. generation when inside a transaction (by opening a new connection). note: the portal make uses of such a transaction. --- core/counter.class.inc.php | 134 ++++++++++++++++++++++++------ core/dbobjectsearch.class.php | 18 ++++ core/sqlobjectquery.class.inc.php | 35 +++++++- 3 files changed, 159 insertions(+), 28 deletions(-) diff --git a/core/counter.class.inc.php b/core/counter.class.inc.php index 93c4a3c51..013e658cc 100644 --- a/core/counter.class.inc.php +++ b/core/counter.class.inc.php @@ -24,6 +24,7 @@ */ final class ItopCounter { + /** * Key based counter. * The counter is protected against concurrency script. @@ -35,13 +36,9 @@ final class ItopCounter * * `0` when no $oNewObjectValueProvider is given (or null) * * `$oNewObjectValueProvider() + 1` otherwise * - * @throws \ArchivedObjectException - * @throws \CoreCannotSaveObjectException * @throws \CoreException - * @throws \CoreOqlMultipleResultsForbiddenException - * @throws \CoreUnexpectedValue * @throws \MySQLException - * @throws \OQLException + * @throws \Exception */ public static function Inc($sCounterName, $oNewObjectValueProvider = null) { @@ -50,35 +47,92 @@ final class ItopCounter $oiTopMutex = new iTopMutex($sMutexKeyName); $oiTopMutex->Lock(); - $oFilter = DBObjectSearch::FromOQL('SELECT KeyValueStore WHERE key_name=:key_name AND namespace=:namespace', array( - 'key_name' => $sCounterName, - 'namespace' => $sSelfClassName, - )); - $oCounter = $oFilter->GetFirstResult(); - if (is_null($oCounter)) + $bIsInsideTransaction = CMDBSource::IsInsideTransaction(); + if ($bIsInsideTransaction) { - if (null != $oNewObjectValueProvider) + // # Transaction isolation hack: + // When inside a transaction, we need to open a new connection for the counter. + // So it is visible immediately to the connections outside of the transaction. + // Either way, the lock is not long enought, and there would be duplicate ref. + // + // SELECT ... FOR UPDATE would have also worked but with the cost of extra long lock (until the commit), + // we did not wanted this! As opening a short connection is less prone to starving than a long running one. + // Plus it would trigger way more deadlocks! + $hDBLink = self::InitMySQLSession(); + } + else + { + $hDBLink = CMDBSource::GetMysqli(); + } + + try + { + $oFilter = DBObjectSearch::FromOQL('SELECT KeyValueStore WHERE key_name=:key_name AND namespace=:namespace', array( + 'key_name' => $sCounterName, + 'namespace' => $sSelfClassName, + )); + $oAttDef = MetaModel::GetAttributeDef('KeyValueStore', 'value'); + $aAttToLoad = array('KeyValueStore' => array('value' => $oAttDef)); + $sSql = $oFilter->MakeSelectQuery(array(), array(), $aAttToLoad); + $hResult = mysqli_query($hDBLink, $sSql); + $aCounter = mysqli_fetch_array($hResult, MYSQLI_NUM); + mysqli_free_result($hResult); + + //Rebuild the filter, as the MakeSelectQuery polluted the orignal and it cannot be reused + $oFilter = DBObjectSearch::FromOQL('SELECT KeyValueStore WHERE key_name=:key_name AND namespace=:namespace', array( + 'key_name' => $sCounterName, + 'namespace' => $sSelfClassName, + )); + + if (is_null($aCounter)) { - $iComputedValue = $oNewObjectValueProvider(); + if (null != $oNewObjectValueProvider) + { + $iComputedValue = $oNewObjectValueProvider(); + } + else + { + $iComputedValue = 0; + } + + $iCurrentValue = $iComputedValue + 1; + + $aQueryParams = array( + 'key_name' => $sCounterName, + 'value' => "$iCurrentValue", + 'namespace' => $sSelfClassName, + ); + + $sSql = $oFilter->MakeInsertQuery($aQueryParams); } else { - $iComputedValue = 0; + $iCurrentValue = (int) $aCounter[1]; + $iCurrentValue++; + $aQueryParams = array( + 'value' => "$iCurrentValue", + ); + + $sSql = $oFilter->MakeUpdateQuery($aQueryParams); } - $oCounter = MetaModel::NewObject('KeyValueStore', array( - 'key_name' => $sCounterName, - 'value' => $iComputedValue, - 'namespace' => $sSelfClassName, - )); + + $hResult = mysqli_query($hDBLink, $sSql); + mysqli_free_result($hResult); + + } + catch(Exception $e) + { + IssueLog::Error($e->getMessage()); + throw $e; + } + finally + { + if ($bIsInsideTransaction) + { + mysqli_close($hDBLink); + } + $oiTopMutex->Unlock(); } - - $iCurrentValue = (int) $oCounter->Get('value'); - $iCurrentValue++; - - $oCounter->Set('value', $iCurrentValue); - $oCounter->DBWrite(); - - $oiTopMutex->Unlock(); return $iCurrentValue; } @@ -114,6 +168,32 @@ final class ItopCounter return self::Inc($sRootClass, $oNewObjectCallback); } + + /** + * @return \mysqli + * @throws \ConfigException + * @throws \CoreException + * @throws \MySQLException + */ + private static function InitMySQLSession() + { + $oConfig = utils::GetConfig(); + $sDBHost = $oConfig->Get('db_host'); + $sDBUser = $oConfig->Get('db_user'); + $sDBPwd = $oConfig->Get('db_pwd'); + $sDBName = $oConfig->Get('db_name'); + $bDBTlsEnabled = $oConfig->Get('db_tls.enabled'); + $sDBTlsCA = $oConfig->Get('db_tls.ca'); + + $hDBLink = CMDBSource::GetMysqliInstance($sDBHost, $sDBUser, $sDBPwd, $sDBName, $bDBTlsEnabled, $sDBTlsCA, false); + + if (!$hDBLink) + { + throw new Exception("Could not connect to the DB server (host=$sDBHost, user=$sDBUser): ".mysqli_connect_error().' (mysql errno: '.mysqli_connect_errno().')'); + } + + return $hDBLink; + } } diff --git a/core/dbobjectsearch.class.php b/core/dbobjectsearch.class.php index ba633b4e0..d5a2fc848 100644 --- a/core/dbobjectsearch.class.php +++ b/core/dbobjectsearch.class.php @@ -1798,6 +1798,24 @@ class DBObjectSearch extends DBSearch return $sRet; } + /** + * Generate an INSERT statement. + * Note : unlike `RenderUpdate` and `RenderSelect`, it is limited to one and only one table. + * + * @param array $aValues is an array of $sAttCode => $value + * @param array $aArgs + * + * @return string + * @throws \CoreException + */ + public function MakeInsertQuery($aValues, $aArgs = array()) + { + $oSQLObjectQueryBuilder = new SQLObjectQueryBuilder($this); + $oSQLQuery = $oSQLObjectQueryBuilder->MakeSQLObjectUpdateQuery($aValues); + $aScalarArgs = MetaModel::PrepareQueryArguments($aArgs, $this->GetInternalParams()); + $sRet = $oSQLQuery->RenderInsert($aScalarArgs); + return $sRet; + } /** * Get an SQLObjectQuery from the search. This SQLObjectQuery can be rendered as a select, select group by, update or delete diff --git a/core/sqlobjectquery.class.inc.php b/core/sqlobjectquery.class.inc.php index 6ee8c2900..38f767303 100644 --- a/core/sqlobjectquery.class.inc.php +++ b/core/sqlobjectquery.class.inc.php @@ -318,7 +318,40 @@ class SQLObjectQuery extends SQLQuery return "UPDATE $sFrom SET $sValues WHERE $sWhere"; } - // Interface, build the SQL query + + /** + * Generate an INSERT statement. + * Note : unlike `RenderUpdate` and `RenderSelect`, it is limited to one and only one table. + * + * + * @param array $aArgs + * @return string + * @throws CoreException + */ + public function RenderInsert($aArgs = array()) + { + $this->PrepareRendering(); + $aJoinInfo = reset($this->__aFrom); + + if ($aJoinInfo['jointype'] != 'first' || count($this->__aFrom) > 1) + { + throw new CoreException('Cannot render insert'); + } + + $sFrom = "`{$aJoinInfo['tablename']}`"; + + $sColList = '`'.implode('`,`', array_keys($this->m_aValues)).'`'; + + $aSetValues = array(); + foreach ($this->__aSetValues as $sFieldSpec => $value) + { + $aSetValues[] = CMDBSource::Quote($value); + } + $sValues = implode(',', $aSetValues); + + return "INSERT INTO $sFrom ($sColList) VALUES ($sValues)"; + } + /** * @param array $aOrderBy