U4-10117 - Getting a dictionary item may not be cached properly

Created by Sebastiaan Janssen 05 Jul 2017, 14:15:43 Updated by Claus Jensen 07 Jul 2017, 11:34:31

Tags: Unscheduled

Subtask of: U4-9609

Was profiling a site earlier with a lot of dictionary items on the frontend and it seems like on each page refresh it would get the dictionary items from the database instead of cache.

The cache potentially never worked for the get by key method.

1 Attachments

Comments

Shannon Deminick 06 Jul 2017, 02:15:38

Findings:

Getting by Key is cached, so I'm not sure how you are running your profiling and if you are always using the same Key to lookup the item. If you put a breakpoint on DictionaryByKeyRepository.PerformFetch you'll see this isn't hit a second time for the same dictionary item being queried.

On another note, the reason why the SQL lookup is slow is because we're missing a very important index in the database which is on the key column of the cmsDictionary table since this is what GetByKey queries against. But we cannot add this index because currently the key column is a 1000 char limit which is far too big to index. The max size column that can exist that is indexed is 900 bytes and with nvarchar this equates to 450 chars.

I don't see why we would need a dictionary key to be so big, even 450 is pretty huge but. In 7.7 we can resize this column (if there isn't anything already too big in there) and then add the index which will definitely boost the querying by key.

In the meantime I need to understand if you are in fact seeing the DictionaryByKeyRepository.PerformFetch hit on the website when using the same key.

For a workaround for customers for now, they can check the max size of the value in the column and if it's less than 900 bytes, they can resize the db column to 450 and then add an index. This is done by this SQL (NOTE: This was generated by SQL management studio)

/******** FIRST CHECK THE MAX byte SIZE FOR THE COL ******/

select max(datalength([key])) from cmsDictionary

Make sure this is less than 900. If it is you can proceed with the below script. If it is bigger than there's not much we can do except have them change how their dictionary is structured to be less nested so that key lengths are smaller.


/*********** 
This will alter the column size to 450 and add the index while preserving existing data
************/


/* To prevent any potential data loss issues, you should review this script in detail before running it outside the context of the database designer.*/
BEGIN TRANSACTION
SET QUOTED_IDENTIFIER ON
SET ARITHABORT ON
SET NUMERIC_ROUNDABORT OFF
SET CONCAT_NULL_YIELDS_NULL ON
SET ANSI_NULLS ON
SET ANSI_PADDING ON
SET ANSI_WARNINGS ON
COMMIT
BEGIN TRANSACTION
GO
CREATE TABLE dbo.Tmp_cmsDictionary
	(
	pk int NOT NULL IDENTITY (1, 1),
	id uniqueidentifier NOT NULL,
	parent uniqueidentifier NULL,
	[key] nvarchar(450) NOT NULL
	)  ON [PRIMARY]
GO
ALTER TABLE dbo.Tmp_cmsDictionary SET (LOCK_ESCALATION = TABLE)
GO
SET IDENTITY_INSERT dbo.Tmp_cmsDictionary ON
GO
IF EXISTS(SELECT * FROM dbo.cmsDictionary)
	 EXEC('INSERT INTO dbo.Tmp_cmsDictionary (pk, id, parent, [key])
		SELECT pk, id, parent, CONVERT(nvarchar(450), [key]) FROM dbo.cmsDictionary WITH (HOLDLOCK TABLOCKX)')
GO
SET IDENTITY_INSERT dbo.Tmp_cmsDictionary OFF
GO
ALTER TABLE dbo.cmsDictionary
	DROP CONSTRAINT FK_cmsDictionary_cmsDictionary_id
GO
ALTER TABLE dbo.cmsLanguageText
	DROP CONSTRAINT FK_cmsLanguageText_cmsDictionary_id
GO
DROP TABLE dbo.cmsDictionary
GO
EXECUTE sp_rename N'dbo.Tmp_cmsDictionary', N'cmsDictionary', 'OBJECT' 
GO
ALTER TABLE dbo.cmsDictionary ADD CONSTRAINT
	PK_cmsDictionary PRIMARY KEY CLUSTERED 
	(
	pk
	) WITH( STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]

GO
CREATE UNIQUE NONCLUSTERED INDEX IX_cmsDictionary_id ON dbo.cmsDictionary
	(
	id
	) WITH( STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
GO
CREATE NONCLUSTERED INDEX IX_cmsDictionary_key ON dbo.cmsDictionary
	(
	[key]
	) WITH( STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
GO
ALTER TABLE dbo.cmsDictionary ADD CONSTRAINT
	FK_cmsDictionary_cmsDictionary_id FOREIGN KEY
	(
	parent
	) REFERENCES dbo.cmsDictionary
	(
	id
	) ON UPDATE  NO ACTION 
	 ON DELETE  NO ACTION 
	
GO
COMMIT
BEGIN TRANSACTION
GO
ALTER TABLE dbo.cmsLanguageText ADD CONSTRAINT
	FK_cmsLanguageText_cmsDictionary_id FOREIGN KEY
	(
	UniqueId
	) REFERENCES dbo.cmsDictionary
	(
	id
	) ON UPDATE  NO ACTION 
	 ON DELETE  NO ACTION 
	
GO
ALTER TABLE dbo.cmsLanguageText SET (LOCK_ESCALATION = TABLE)
GO
COMMIT


Shannon Deminick 06 Jul 2017, 02:46:56

I've made a PR for 7.7 to add this db index: https://github.com/umbraco/Umbraco-CMS/pull/2031


Sebastiaan Janssen 06 Jul 2017, 13:29:59

Great! I've just realized why my cache doesn't work... * facepalm *

It's because I'm looking up dictionary items.. in an empty database, so there are no translations, so each time it checks again if there's anything available in the db.


Shannon Deminick 07 Jul 2017, 00:30:43

AH yes, there is a setting for that in our dictionary repo to not cache empty results, i can see if we should change that.


Priority: Normal

Type: Bug

State: Fixed

Assignee:

Difficulty: Normal

Category:

Backwards Compatible: True

Fix Submitted:

Affected versions: 7.5.14

Due in version: 7.7.0

Sprint: Sprint 62

Story Points:

Cycle: 3