Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for disabling loading all site collection term groups #1069

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

fzbm
Copy link
Contributor

@fzbm fzbm commented Oct 8, 2024

This pull requests adds the support to disable the loading of all site collection-related term groups when initializing the TokenParser.

Currently TokenParser.AddTermStoreTokens would load all available term groups from the term store when at least one termsetid token was found. Yesterday (2024-10-07) at 9:21 UTC we encountered the first instance of a timeout issue on one larger tenant, which was clearly caused by the AddTermStoreTokens method of TokenParser due to the stack trace. The request was canceled after three minutes.

System.Threading.Tasks.TaskCanceledException: A task was canceled.
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   ...
   at Microsoft.SharePoint.Client.ClientContextExtensions.ExecuteQueryRetry(ClientRuntimeContext clientContext, Int32 retryCount, String userAgent)
   at PnP.Framework.Provisioning.ObjectHandlers.TokenParser.AddTermStoreTokens(Web web, List`1 tokenIds)
   at PnP.Framework.Provisioning.ObjectHandlers.TokenParser..ctor(Web web, ProvisioningTemplate template, ProvisioningTemplateApplyingInformation applyingInformation)
   at PnP.Framework.Provisioning.ObjectHandlers.SiteToTemplateConversion.ApplyRemoteTemplate(Web web, ProvisioningTemplate template, ProvisioningTemplateApplyingInformation provisioningInfo, Boolean calledFromHierarchy, TokenParser tokenParser)
   ...

After some manual tests we figured out that loading all available term groups with their term sets caused the issue. Removing the term sets from the loading process solved the problem, but would defeat the purpose of the whole operation. During those tests we also realized that loading the term groups without any conditions would return (on that tenant) over 29,000 entries. Most of them were site collection term groups. The same operation on another large tenant finished after a few seconds, but returned only around 28,800 entries. So we don't know if this is just an issue on that one specific tenant, or if 29,000 is somehow a magic barrier and the other tenant is just on the edge to experience the same issue.

This could also be caused by one or more site collection term groups, which contain a larger amount of term sets, but we have not checked this yet. It also does not make in most cases sense for PnP to load all term groups including the site collection ones, because most templates might contain a reference to the site collection term group of the site it gets applied to, but not references to those from other site collections. Excluding them also greatly reduces the amount of tokens the TokenParser has to handle.

To remedy this issue we added the LoadSiteCollectionTermGroups property to the ProvisioningTemplateApplyingInformation and ProvisioningTemplateCreationInformation class. It is true by default to ensure the same behavior as before. Changing it to false will change the query with which the term groups get loaded from a normal Load to a LoadQuery, that excludes all groups which are related to a site collection (IsSiteCollectionGroup property). The sitecollectionterm token are not affected by this change and AddTermStoreTokens will still load information about the term group of the current site collection. Beside the TokenParser class ObjectTermGroups and ObjectHierarchySequenceTermGroups have been changed too, because they also load all term groups and would cause the same issue.

We tested the changes and deployed them already to our production environment using a reference to the project instead of using the NuGet package, because it was no longer possible to deploy any templates due to the timeout issue. We haven't seen any issues related to the changes yet. The hierarchy-related logic has also been changed, but because we don't use it it was not easily possible to test.

It is now possible to specify which term groups should be loaded when
initializing the token parser. By default all groups including site
collection specific ones are loaded. If disabled, only normal term
groups will be loaded, which reduces the amount of data to retrieve from
SharePoint.
A "LoadSiteCollectionTermGroups" property has been added to the
"ApplyConfiguration" class.

Additionally various usages of the "TokenParser" class have been updated
to now also provide the template applying information.
"ObjectTermGroups" and "ObjectHierarchySequenceTermGroups" now also
respect "LoadSiteCollectionTermGroups" when loading term groups.
@fzbm fzbm marked this pull request as ready for review October 8, 2024 06:06
@fzbm
Copy link
Contributor Author

fzbm commented Oct 11, 2024

It looks like that this is a general issue, which also affects the frontend. At least the management part of the term store (_layouts/15/SiteAdmin.aspx). Loading the Site level term groups takes even on smaller tenants with a few hundred sites 60 seconds or more. Most of the times the request even runs into an error with the server returning an error about using too many resources.

{
    "error": {
        "code": "generalException",
        "message": "The request uses too many resources."
    }
}

The management page previously never returned all site collection term groups. Only the one for the current site. Something definitely changed starting last monday.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant