I have recently noticed performance issues for the main application I am developing right now (it is a legacy one). A job triggers a stored procedure that computes the security for all the documents and all the application users in the database. The table looks like the following:
Id INT IDENTITY(1, 1) CONSTRAINT PK_DocumentSecurity PRIMARY KEY CLUSTERED, DocumentId INT CONSTRAINT FK_DocumentSecurity FOREIGN KEY REFERENCES Document, UserId INT CONSTRAINT FK_DocumentSecurity FOREIGN KEY REFERENCES ApplicationUser, CanWrite BIT NOT NULL, + an index for DocumentId and UserId
This table has about 3.6M records in production, but this keeps growing due to the increasing number of documents and users.
This happens very often (once a minute) and I have noticed using sp_who2 that it sometimes blocks other SPIDs. The profiler also indicates a very large number of reads and CPU, so clearly, this process is very heavy.
Sometimes, on the test environments, the computation seems to take forever, probably due to a very inefficient query plan that happens from time to time (test servers are significantly lower on resources than the production one).
I have checked the code and the current algorithm looks like the following:
- compute a hash for all the records in the tables involved in the computation
- if the hash is the same as before, do nothing
- insert the result of a big and complicated SELECT statement (I will provide details below) to insert into a persistent table having the exact schema as above
- when done the view used for joining to apply the security is altered to use the brand new computed table
The actual computation looks like the following (actual details are removed due to brevity):
SELECT DISTINCT ISNULL(ds.UserId, 0), ISNULL(ds.DocumentId, 0) FROM ( SELECT ... FROM Document ... CROSS JOIN ApplicationUser ... UNION SELECT ... FROM Document ... CROSS JOIN ApplicationUserRole ... UNION SELECT ... FROM Document ... UNION SELECT DISTINCT ... FROM DocumentDetail ... UNION ) ds JOIN ( SELECT ... FROM Document ... WHERE (... OR ... OR ... ) AND ... UNION SELECT ... ) dsp ON dsp.DocumentId = ds.DocumentId AND dsp.UserId = ds.UserId
From my perspective, there are two big problems with this way of computation:
- maintenance: it is becoming harder and harder to introduce new security rules
- performance: ORs and DISTINCTs are killing the performance
I have already created a story to refactor this code, but I would like to validate that my reasoning is correct (I have an OOP bias, as I am mainly a .NET developer) before starting working on it:
- refactor the security rules (to be validated by the business, since they certainly look different than what is already there which also might not be 100% correct for all cases) that they all like the following (precedence matter here):
if role is admin, allow access for all documents if role is sales and
user has read role for document, insert with CanWrite = 0 if role is
sales and user has a write role for document, insert with CanWrite = 1
create a temporary table with the target schema
create an index for DocumentId and UserId
apply each security rule:
INSERT INTO #buffer (DocumentId, UserId, CanWrite) SELECT ... FROM Document D ... JOIN ApplicationUser U ... WHERE NOT EXISTS (SELECT 1 FROM #buffer B WHERE B.DocumentId = D.DocumentId AND B.UserId = U.UserId)
truncate the target table
INSERT INTO DocumentSecurity SELECT * FROM #buffer
The last two steps might need to be optimized if they take too long (maybe replace with the swap the table under the view mechanism already in place).
This should make the code more maintainable and I assume the performance will be better if I remove all the ORs and the DISTINCTs, but I am interested in a DBA's perspective for such an approach. Is it suboptimal?
Note: I cannot have duplicates in the security table because there are currently many places (Entity Framework used in application layer + stored procedures) that simply JOIN with the security view (as opposed to check for existence).