Jump to content

Database performance issues after upgrading from 4.7.13 to 4.7.16 (getItemsWithPermission)

You are viewing a curated collection of the most significant posts in this topic with an estimated read time of 2 minutes. The full topic contains 35 posts with an estimated read time of 33 minutes.

Featured Replies

Posted

I upgraded from version 4.7.13 to 4.7.16 on 19th April, and we have since been having trouble with both reduced performance and more downtime. 

Our server host has looked into this and reported back a major increase in slow queries, major increase in CPU usage for the database server overall and less usage of MySQL Sorts. 

Could contain: Electronics

It seems this increase can be tracked down to methods using IPS\Content\_Item::getItemsWithPermission:317. We have a custom controller and a frontpage widget utilizing this method. 

Anyone else experienced this, or have any tips on what could be going on? Hope Invision could be interested in lending a hand investigating this too 

Best regards, Preben

Edited by TSP

  • Author
31 minutes ago, Sonya* said:

Yes. I have noticed slow performance on my test server.

Thanks for the feedback 🙂

I presume you have 4.7.16 on your test server, and that your production server is still running an earlier version, which version?

 

Between versions 4.7.13 and 4.7.16 I see a bunch of changes in system/Content/Item.php.

However, I don't have a changelog for the in between versions. Do anyone know which versions these changes were introduced? I suspect some of the changes has to do with the change in database server utilization... 

Here are the diff for the changes in the getItemsWithPermission-method in the file system/Content/Item.php: 

@@ -3159,11 +3161,16 @@ abstract class _Item extends \IPS\Content
 				{
 					$containerWhere = array_merge( $containerWhere, $value );
 					unset( $where[ $key ] );
+
+					/* $containerWhere is used for exclusion purposes now,
+					so we leave this as part of the where condition as well. */
+					$where = array_merge( $where, $value );
 				}
 			}
 		}
 		
 		/* Exclude hidden items */
+		$includeAdditionalApprovalClauses = true;
 		if( $includeHiddenItems === \IPS\Content\Hideable::FILTER_AUTOMATIC )
 		{
 			$containersTheUserCanViewHiddenItemsIn = static::canViewHiddenItemsContainers( $member );
@@ -3199,6 +3206,8 @@ abstract class _Item extends \IPS\Content
 				$col = static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['hidden'];
 				$where[] = array( "{$col}=1" );
 			}
+
+			$includeAdditionalApprovalClauses = false;
 		}
 		elseif ( \in_array( 'IPS\Content\Hideable', class_implements( \get_called_class() ) ) and $includeHiddenItems !== \IPS\Content\Hideable::FILTER_SHOW_HIDDEN )
 		{
@@ -3226,6 +3235,8 @@ abstract class _Item extends \IPS\Content
 				{
 					$where[] = array( "{$col}=1" );
 				}
+
+				$includeAdditionalApprovalClauses = false;
 			}
 			elseif ( isset( static::$databaseColumnMap['hidden'] ) )
 			{
@@ -3247,6 +3258,8 @@ abstract class _Item extends \IPS\Content
 				{
 					$where[] = array( "{$col}=0" );
 				}
+
+				$includeAdditionalApprovalClauses = false;
 			}
 		}
         else
@@ -3274,17 +3287,21 @@ abstract class _Item extends \IPS\Content
             }
         }
         
-        /* No matter if we can or cannot view hidden items, we do not want these to show: -2 is queued for deletion and -3 is posted before register */
-        if ( isset( static::$databaseColumnMap['hidden'] ) )
-        {
-	        $col = static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['hidden'];
-	        $where[] = array( "{$col}!=-2 AND {$col} !=-3" );
-        }
-        else if ( isset( static::$databaseColumnMap['approved'] ) )
-        {
-	        $col = static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['approved'];
-	        $where[] = array( "{$col}!=-2 AND {$col}!=-3" );
-        }
+        /* This only makes sense if we have not already filtered by a single value */
+		if( $includeAdditionalApprovalClauses )
+		{
+			/* No matter if we can or cannot view hidden items, we do not want these to show: -2 is queued for deletion and -3 is posted before register */
+			if ( isset( static::$databaseColumnMap['hidden'] ) )
+			{
+				$col = static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['hidden'];
+				$where[] = array( "{$col}!=-2 AND {$col} !=-3" );
+			}
+			else if ( isset( static::$databaseColumnMap['approved'] ) )
+			{
+				$col = static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['approved'];
+				$where[] = array( "{$col}!=-2 AND {$col}!=-3" );
+			}
+		}
         
 		/* Future items? */
 		if ( \in_array( 'IPS\Content\FuturePublishing', class_implements( \get_called_class() ) ) )
@@ -3321,18 +3338,28 @@ abstract class _Item extends \IPS\Content
 			$categories	= array();
 			$lookupKey	= md5( $containerClass::$permApp . $containerClass::$permType . $permissionKey . json_encode( $member->groups ) );
 
+			/* EME: Switch to use exclusion instead of inclusion */
 			if( !isset( static::$permissionSelect[ $lookupKey ] ) )
 			{
 				static::$permissionSelect[ $lookupKey ] = array();
-				$permQuery = \IPS\Db::i()->select( 'perm_type_id', 'core_permission_index', array( "core_permission_index.app='" . $containerClass::$permApp . "' AND core_permission_index.perm_type='" . $containerClass::$permType . "' AND (" . \IPS\Db::i()->findInSet( 'perm_' . $containerClass::$permissionMap[ $permissionKey ], $member->permissionArray() ) . ' OR ' . 'perm_' . $containerClass::$permissionMap[ $permissionKey ] . "='*' )" ) );
+
+				$permQueryJoinContainer = (bool) ( \count( $containerWhere ) );
+				$clubWhere = "";
 
 				/* If we cannot access clubs, skip them */
 				if ( \IPS\IPS::classUsesTrait( $containerClass, 'IPS\Content\ClubContainer' ) AND !$member->canAccessModule( \IPS\Application\Module::get( 'core', 'clubs', 'front' ) ) )
 				{
-					$containerWhere[] = array( $containerClass::$databaseTable . '.' . $containerClass::$databasePrefix . $containerClass::clubIdColumn() . ' IS NULL' );
+					$permQueryWhere = array( "core_permission_index.app='" . $containerClass::$permApp . "' AND core_permission_index.perm_type='" . $containerClass::$permType . "' AND (" . $containerClass::$databaseTable . '.' . $containerClass::$databasePrefix . $containerClass::clubIdColumn() . " IS NOT NULL OR !(" . \IPS\Db::i()->findInSet( 'perm_' . $containerClass::$permissionMap[ $permissionKey ], $member->permissionArray() ) . ' OR ' . 'perm_' . $containerClass::$permissionMap[ $permissionKey ] . "='*' ))" );
+					$permQueryJoinContainer = true;
 				}
+				else
+				{
+					$permQueryWhere = array( "core_permission_index.app='" . $containerClass::$permApp . "' AND core_permission_index.perm_type='" . $containerClass::$permType . "' AND !(" . \IPS\Db::i()->findInSet( 'perm_' . $containerClass::$permissionMap[ $permissionKey ], $member->permissionArray() ) . ' OR ' . 'perm_' . $containerClass::$permissionMap[ $permissionKey ] . "='*' )" );
+				}
+
+				$permQuery = \IPS\Db::i()->select( 'perm_type_id', 'core_permission_index', $permQueryWhere );
 				
-				if ( \count( $containerWhere ) )
+				if( $permQueryJoinContainer )
 				{
 					$permQuery->join( $containerClass::$databaseTable, array_merge( $containerWhere, array( 'core_permission_index.perm_type_id=' . $containerClass::$databaseTable . '.' . $containerClass::$databasePrefix . $containerClass::$databaseColumnId ) ), 'STRAIGHT_JOIN' );
 				}
@@ -3347,11 +3374,7 @@ abstract class _Item extends \IPS\Content
 
 			if( \count( $categories ) )
 			{
-				$where[]	= array( static::$databaseTable . "." . static::$databasePrefix . static::$databaseColumnMap['container'] . ' IN(' . implode( ',', $categories ) . ')' );
-			}
-			else
-			{
-				$where[]	= array( static::$databaseTable . "." . static::$databasePrefix . static::$databaseColumnMap['container'] . '=0' );
+				$where[] = array( static::$databaseTable . "." . static::$databasePrefix . static::$databaseColumnMap['container'] . ' NOT IN (' . implode( ',', $categories ) . ')' );
 			}
 		}
 		
@@ -3363,8 +3386,10 @@ abstract class _Item extends \IPS\Content
 			$select = \IPS\Db::i()->select( 'COUNT(*) as cnt', static::$databaseTable, $where, NULL, NULL, $groupBy, NULL, $queryFlags );
 			if ( $joinContainer AND isset( static::$containerNodeClass ) )
 			{
+				/* EME: Removed the $containerWhere from the join because it is now in the where clause.
+				We are now using $containerWhere to exclude forums that shouldn't be visible. */
 				$containerClass = static::$containerNodeClass;
-				$select->join( $containerClass::$databaseTable, array_merge( $containerWhere, array( static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['container'] . '=' . $containerClass::$databaseTable . '.' . $containerClass::$databasePrefix . $containerClass::$databaseColumnId ) ) );
+				$select->join( $containerClass::$databaseTable, array( static::$databaseTable . '.' . static::$databasePrefix . static::$databaseColumnMap['container'] . '=' . $containerClass::$databaseTable . '.' . $containerClass::$databasePrefix . $containerClass::$databaseColumnId ) );
 			}
 			if ( $joinComments )
 			{

 

Could someone at Invision answer whether it would cause any issues to temporarily revert this method to the previous that worked for us? Or is there a lot of changes in that method dependent on other changes done elsewhere in the same or later version, so I would have to hunt down a lot of other stuff? 

25 minutes ago, TSP said:

I presume you have 4.7.16 on your test server, and that your production server is still running an earlier version, which version?

They both have the same version. But my test server is not optimized and is poor configured. I manage it myself and have no idea of how to do it properly. 😅 But I have noticed that my custom application on the test server that uses exact the same function is now slow.

My live server is well configured by my server admin. So I do not notice any significant difference there. 

  • Author
30 minutes ago, Stuart Silvester said:

What version of MySQL/MariaDB are you using?

Server version: 10.5.24-MariaDB-1:10.5.24+maria~ubu2004-log mariadb.org binary distribution

  • Author

@Stuart Silvester does my reply about database version help at all? Is this a known issue that is being worked on?

  • Community Expert

We're working on a performance review, we've included this information in that project so we can look further into it and do some further benchmarking. We're not seeing the same slowness on our Cloud infrastructure hence my asking about MySQL/MariaDB versions.

  • 3 weeks later...
  • Management
7 minutes ago, Dll said:

I'm not sure that should be used instead of well-written, efficient SQL queries though, and I'm pretty sure there's scope to improve those in this instance.

The search area is very complex with a lot going on. We have plans to improve it post 5.0.0.

  • Management
2 hours ago, Westfield Sports Car Club said:

Removing FORCE INDEX makes a massive difference!

I ran the full query manually and it just hung for over 5 mins, then without FORCE INDEX it ran in an instant - with only a few results though.

EXPLAIN output attached as CSV

forum_WSCC Forum_29-05-2024@12-34.csv 498 B · 4 downloads

Give this plugin a go (it's unsupported, etc, etc if anything goes wrong, just disable it).

Search_ no force index() 1.0.0.xml

  • Author
On 5/29/2024 at 12:55 PM, Matt said:

Do you have topics with thousands of pages of posts? We did remove an old optimisation that helped with older versions of MySQL but caused issues with. MySQL 8.

We have some. 

I've provided more details in a reply I just made to the ticket. 

Recently Browsing 0

  • No registered users viewing this page.