PLAT-9401: Support working with sphinx indexes which are distributed over several machines (split index) #7828

yossipapi · 2018-11-15T08:12:42Z

No description provided.

…distributed over several machines (split index)

…side

…able

kaltura-hooks · 2019-01-29T10:02:25Z

Hi @yossipapi,
Thank you for contributing this pull request!
Please sign the Kaltura CLA so we can review and merge your contribution.
Learn more at http://bit.ly/KalturaContrib

… Naos-14.8.0-PLAT-9401

…function by index generator

… Naos-14.8.0-PLAT-9401

MosheMaorKaltura · 2019-05-15T10:39:16Z

alpha/apps/kaltura/lib/db/DbManager.php

 	{
-		if (!self::$sphinxCache || self::$connIndex === self::$cachedConnIndex)
+		if (!self::$sphinxCache || isset(self::$connIndexes[$indexName]) === isset(self::$cachedConnIndexes[$indexName]))


Did you mean checking if both are set (or both false) , or comparing the names ?

MosheMaorKaltura · 2019-05-15T10:39:25Z

alpha/apps/kaltura/lib/db/DbManager.php

-		KalturaLog::debug("Setting sphinx sticky session for key [" . self::$stickySessionKey . "] to sphinx index [" . self::$connIndex . "]");
-		self::$sphinxCache->set(self::$stickySessionKey, self::$connIndex, $stickySessionExpiry);
-		self::$cachedConnIndex = self::$connIndex;
+		KalturaLog::debug("Setting sphinx sticky session for key [" . self::$stickySessionKey . "] to sphinx index [" . print_r(self::$connIndexes, true) . "]");


Better use single quote since no process is done inside the string,

MosheMaorKaltura · 2019-05-15T10:41:31Z

alpha/apps/kaltura/lib/db/DbManager.php

-		KalturaLog::debug("Got sphinx sticky session for key [" . self::$stickySessionKey . "] to sphinx index [" . $preferredIndex . "]");
-		if ($preferredIndex === false)
+		KalturaLog::debug("Got sphinx sticky session for key [" . self::$stickySessionKey . "] to sphinx index [" . print_r($preferredIndex, true) . "]");
+		if ($preferredIndex === false || !isset($preferredIndex[$indexName]))


Coding standard use {}

MosheMaorKaltura · 2019-05-15T10:46:04Z

alpha/apps/kaltura/lib/db/DbManager.php

 	}

-	protected static function getSphinxConnIndexFromCache()
+	protected static function getSphinxConnIndexFromCache($indexName = null)


Function name implies on get , but it changes the state by updating - self::$cachedConnIndexes.
Rename function of break to 2 sub functions - getSphinxConnIndexFromCache and Update

MosheMaorKaltura · 2019-05-15T10:47:12Z

alpha/apps/kaltura/lib/db/DbManager.php

@@ -233,35 +233,40 @@ protected static function getStickySessionKey()
 	/**
 	 * @return KalturaPDO
 	 */
-	public static function getSphinxConnection($read = true)
+	public static function getSphinxConnection($read = true, $indexName = null)


Why not set default indexName to 'default_index'?

MosheMaorKaltura · 2019-05-15T11:14:45Z

deployment/base/scripts/index/IndexGeneratorBase.php

+		$object->setIndexName($indexName);
+
+		$splitIndexFieldName = null;
+		if(isset($objectAttribtues["splitIndexFieldName"]))


MosheMaorKaltura · 2019-05-15T11:15:07Z

deployment/base/scripts/index/IndexGeneratorBase.php

+
+		$splitIndexFieldName = null;
+		if(isset($objectAttribtues["splitIndexFieldName"]))
+			$splitIndexFieldName = $indexName . "." . $objectAttribtues["splitIndexFieldName"];


Better use const for splitIndexFieldName

MosheMaorKaltura · 2019-05-15T11:16:01Z

plugins/metadata/lib/model/Metadata.php

+	 */
+	public function getEntry()
+	{
+		if ($this->getObjectType() == MetadataObjectType::ENTRY)


MosheMaorKaltura · 2019-05-15T11:19:38Z

alpha/lib/interfaces/IIndexable.php

+	/**
+	 * This function returns the index name to use
+	 */
+	public function getSphinxIndexName();


This function can be implemented once instead of 13 times in an abstract class that all relevant classes will inherit from , and it will inherit baseObject.
function getSphinxIndexName()
{
$IndexClass = $this->getIndexObjectName();
return kSphinxSearchManager::getSphinxIndexName($IndexClass::getObjectIndexName());
}

MosheMaorKaltura · 2019-05-15T11:19:58Z

plugins/search/providers/sphinx_search/scripts/populateFromLog.php

+
+	foreach ($sphinxTablesData as $sphinxTableData)
+	{
+		if($sphinxTableData['Type'] == "rt")


… Naos-14.8.0-PLAT-9401

MosheMaorKaltura · 2019-05-19T07:56:01Z

alpha/apps/kaltura/lib/db/DbManager.php

-		KalturaLog::debug("Setting sphinx sticky session for key [" . self::$stickySessionKey . "] to sphinx index [" . self::$connIndex . "]");
-		self::$sphinxCache->set(self::$stickySessionKey, self::$connIndex, $stickySessionExpiry);
-		self::$cachedConnIndex = self::$connIndex;
+		KalturaLog::debug("Setting sphinx sticky session for key [" . self::$stickySessionKey . "] to sphinx index [" . print_r(self::$connIndexes, true) . "]");


Use single ' since no processing

MosheMaorKaltura · 2019-05-19T07:58:28Z

alpha/apps/kaltura/lib/db/DbManager.php

 		{
-			$sphinxDS = isset(self::$config['sphinx_datasources']['datasources']) ? self::$config['sphinx_datasources']['datasources'] : array(self::DB_CONFIG_SPHINX);
+			if($indexName && isset(self::$config['sphinx_datasources_'.$indexName]) && isset(self::$config['sphinx_datasources_'.$indexName]['datasources']))


No need to check - isset(self::$config['sphinx_datasources_'.$indexName]), isset will return false to $config['sphinx_datasources_'.$indexName]['datasources'] even if self::$config['sphinx_datasources_'.$indexName] no exist.

MosheMaorKaltura · 2019-05-19T07:58:39Z

alpha/apps/kaltura/lib/db/DbManager.php

 		{
-			$sphinxDS = isset(self::$config['sphinx_datasources']['datasources']) ? self::$config['sphinx_datasources']['datasources'] : array(self::DB_CONFIG_SPHINX);
+			if($indexName && isset(self::$config['sphinx_datasources_'.$indexName]) && isset(self::$config['sphinx_datasources_'.$indexName]['datasources']))
+				$sphinxDS = self::$config['sphinx_datasources_'.$indexName]['datasources'];


MosheMaorKaltura · 2019-05-19T08:01:55Z

alpha/apps/kaltura/lib/db/DbManager.php

+			if($indexName && isset(self::$config['sphinx_datasources_'.$indexName]) && isset(self::$config['sphinx_datasources_'.$indexName]['datasources']))
+				$sphinxDS = self::$config['sphinx_datasources_'.$indexName]['datasources'];
+			else
+				$sphinxDS = isset(self::$config['sphinx_datasources']['datasources']) ? self::$config['sphinx_datasources']['datasources'] : array(self::DB_CONFIG_SPHINX);


if isset(self::$config['sphinx_datasources_'.$indexName]['datasources']) is true, but it has empty value, you will not reach the else.
Better add isset(self::$config['sphinx_datasources_'.$indexName]['datasources']) to the condition above , and if it fails , set array array(self::DB_CONFIG_SPHINX)

MosheMaorKaltura · 2019-05-19T08:03:39Z

alpha/apps/kaltura/lib/db/DbManager.php

 			{
 				throw new Exception('Failed to connect to any Sphinx config');
 			}
-			KalturaLog::debug("Actual sphinx index [". self::$connIndex. "] sphinx index by best lag [" . $preferredIndex. "]");
+			KalturaLog::debug("Actual sphinx index [". self::$connIndexes[$indexName]. "] sphinx index by best lag [" . $preferredIndex. "]");


prefer ' ( no processing )

MosheMaorKaltura · 2019-05-19T08:05:33Z

alpha/lib/index/BaseIndexObject.php

+
+		$splitIndexFactor = self::getSplitIndexFactor($IndexObjectName);
+		if(!$splitIndexFactor)
+			return null;


MosheMaorKaltura · 2019-05-19T08:07:06Z

alpha/lib/index/BaseIndexObject.php

+
+	}
+
+	public static function getSphinxSplitIndexId($originalValue = null, $IndexObjectName)


Is there a case where $originalValue can be null? or will be called without values? If so why originalValue with default value is not last

MosheMaorKaltura · 2019-05-19T08:07:37Z

deployment/base/scripts/index/IndexObjectsGenerator.php

@@ -307,6 +308,10 @@ private function getSphinxIdField($fp, IndexableObject $object) {
 		$this->printToFile($fp, "return '" . $object->indexId . "';",2);
 	}

+	private function getSphinxSplitIndexFieldName($fp, IndexableObject $object) {


jump line - {

MosheMaorKaltura · 2019-05-19T08:07:55Z

deployment/base/scripts/index/IndexableObject.php

+	/**
+	 * @param string splitIndexFieldName
+	 */
+	public function setSplitIndexFieldName($splitIndexFieldName) {


MosheMaorKaltura · 2019-05-19T08:10:03Z

plugins/search/providers/sphinx_search/lib/kSphinxSearchManager.php

 	{
-		return self::SPHINX_INDEX_NAME . '_' . $baseName;
+		$res = self::SPHINX_INDEX_NAME . '_' . $baseName;
+		if(isset($splitIndexValue))


yossipapi added 7 commits November 15, 2018 10:11

Naos-14.8.0-PLAT-9401: Support working with sphinx indexes which are …

fe17359

…distributed over several machines (split index)

PLAT-9401: Remove test code + modify log message

7a1bea2

PLAT-9401: Extend db.template.ini to contain a more full example

a32a228

PLAT-9401: Add missing sphinx_log column to sphinx CE tables

3155922

Bug Fixes + support running regular & sharded sphinx servers side by …

e579aa4

…side

PLAT-9401: Add support for working with sharded indices

573678c

Add deployment script that adds index name column to the sphinx log t…

12bae86

…able

yossipapi changed the base branch from Naos-14.8.0 to Naos-14.13.0 January 29, 2019 10:00

yossipapi added 3 commits January 29, 2019 12:03

Merge branch 'Naos-14.13.0' of https://github.com/kaltura/server into…

77fdfa8

… Naos-14.8.0-PLAT-9401

Make split index logic shared and add support for generating missing …

29e9adf

…function by index generator

Merge Naos-14.14.0

5fa7580

yossipapi changed the base branch from Naos-14.13.0 to Naos-14.14.0 February 17, 2019 13:29

yossipapi added 2 commits February 17, 2019 16:49

Fixes after testing

5599260

Merge branch 'Naos-14.18.0' of https://github.com/kaltura/server into…

b29b42f

… Naos-14.8.0-PLAT-9401

yossipapi changed the base branch from Naos-14.14.0 to Naos-14.18.0 April 7, 2019 10:55

yossipapi changed the base branch from Naos-14.18.0 to Naos-14.19.0 April 28, 2019 12:24

Merge branch 'Naos-14.20.0' into Naos-14.8.0-PLAT-9401

d89a44b

yossipapi changed the base branch from Naos-14.19.0 to Naos-14.20.0 April 29, 2019 10:16

PLAT-9401: pass splitIndexFieldName when calling setSplitIndexFieldName

6aa47a6

MosheMaorKaltura reviewed May 15, 2019

View reviewed changes

yossipapi changed the base branch from Naos-14.20.0 to Orion-15.0.0 May 15, 2019 14:02

yossipapi added 3 commits May 16, 2019 11:39

PLAT-9401: Fix code review comments

8b098cf

Merge branch 'Orion-15.0.0' of https://github.com/kaltura/server into…

2053f7b

… Naos-14.8.0-PLAT-9401

PLAT-9401: Revert return type change

339bcd5

MosheMaorKaltura reviewed May 19, 2019

View reviewed changes

PLAT-9401: Tidy up the code

7b40c8c

yossipapi merged commit 3310eb0 into Orion-15.0.0 May 19, 2019

yossipapi deleted the Naos-14.8.0-PLAT-9401 branch May 19, 2019 08:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PLAT-9401: Support working with sphinx indexes which are distributed over several machines (split index) #7828

PLAT-9401: Support working with sphinx indexes which are distributed over several machines (split index) #7828

yossipapi commented Nov 15, 2018

kaltura-hooks commented Jan 29, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 15, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019

MosheMaorKaltura May 19, 2019


		}

		public static function getSphinxSplitIndexId($originalValue = null, $IndexObjectName)

PLAT-9401: Support working with sphinx indexes which are distributed over several machines (split index) #7828

PLAT-9401: Support working with sphinx indexes which are distributed over several machines (split index) #7828

Conversation

yossipapi commented Nov 15, 2018

kaltura-hooks commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment