Mfcttrf

Transactional database used for booking things...

Our vendor was asked to replace #temptables with @tablevariables (because of heavy compile locks) but instead they replaced with an actual table that adds SPID as a column to ensure the stored procedure only acts on the applicable rows.

Do you see any risk in this method of operation? Before all transactions were isolated within their own transaction... I worried we may end up locking this table a bunch but their opinion is that SQL uses row-level locking and this won't create more locks.

CREATE TABLE dbo.qryTransactions (
 ID int IDENTITY (0,1) NOT NULL CONSTRAINT pk_qryTransactions PRIMARY KEY CLUSTERED,
 spid int NOT NULL,
 OrderID int,
 ItemID int,
 TimeTransactionStart datetime,
 TimeTransactionEnd datetime,
...other fields
 )

CREATE INDEX idx_qryTransactions_spidID ON qryTransactions (spid, ID) INCLUDE (ItemID, OrderID, TimeTransactionStart, TimeTransactionEnd)

It seems to me using the @@SPID like that is asking for trouble.

Session ID's are reused frequently; as soon as a user connection logs out, that session ID is available to be used again, and is likely to be used by the next session attempting to connect.

To make it work at least semi-reliably, you'd need a login trigger that purges prior rows from the table with the same @@SPID. If you do that, you're likely to see a lot of locking on the table using the @@SPID column.

SQL Server does indeed use row locking, but it also uses page locking, and table locking. Of course, you might be able to mitigate that via good indexing, but this still looks like an anti-pattern to me.

If the stored procedure is the only method used to access the affected tables, you could investigate using an application lock, via sp_getapplock to essentially serialize access to the relevant parts. Docs for sp_getapplock are here. Erik Darling has an interesting post about it here.

Yes, I do see risks. It is naive to count on SQL using Row Locking. For example, I'm pretty sure inserts and deletes will use page locks at least. SQL Engine chooses the lock type based on several factors and none of those factors include "their opinion".
Blanket solutions like changing temp tables to table variables are generally bad ideas also.
Table Variables are very useful in some situations but they do have limitations and performance issues. I prefer temp tables in most circumstances. Particularly when the table will hold more than a few dozen rows.
I would require the vendor to explain why the system experienced "heavy compile locks" and how that degraded performance. Remember, anytime you look you will find "heavy locks" of some sort. That does not necessarily mean the locks are a problem.
Max's comments about @@SPID are also important.
Additionally, the transaction model and error processing could be big issues. If your system experiences deadlocks or input data quality issues then the standard error processing can result in the session being terminated without the qryTransactions table being properly reset.
IMO the wrong solution approach to the original problem.

A bit more rambling than can fit in a comment block ... and want to highlight a comment the OP made in response to Ray's answer:

• parent proc (Common_InsertOrders_1) creates a temp table


• a child proc (InsertOrders) queries the temp table


• compilation locks are being seen for the child proc (InsertOrders)

Going off on a slight tangent for a minute ... on what would happen with this scenario is Sybase ASE ...

• each temp table gets a unique object id (sure, the object id could be re-used at some point but this is rare, and certainly won't happen for concurrent sessions)


• Sybase ASE will normally force a recompile on each execution of the child proc because of the change in object id for the temp table


• Sybase ASE will also force a recompile of the child proc if it sees that the structure of the temp table has changed (eg, different number of columns, different column names/datatypes/nullability) between stored proc invocations


• more recent versions of Sybase ASE have a configuration that (effectively) tells the compiler to ignore changes in temp table object ids thus eliminating the recompilation of the child proc (NOTE: recompilations will still occur if the table structure changes)

Back to the OP's issue (compilation locks on the child proc) ...

• is there a chance that some vestiges of the Sybase ASE behavior could still reside in SQL Server (from when the two products were peas in a pod)?


• are there any SQL Server configurations that could reduce (eliminate?) the recompilations of the child proc (if due to changes in object id)?


• can the OP verify that the parent proc is creating the temp table with the same exact structure/DDL each time?

As for the idea of using a single permanent table with @@SPID to differentiate rows between sessions ... been there, seen that ... yuck; recurring issues:

• how/when to clean up of orphaned rows


• re-use of the @@SPID by the database engine could lead to data accuracy issues if orphaned data exists (or during cleanup of orphaned data, eg, delete where @@SPID=10 but there's a new/current/active session with @@SPID=10 => the cleanup deletes too much data)


• potential for lock escalation from row locks to page/table locks


• if the table has indexes then potential (b)locking when updating the indexes


• depending on which database the table resides in you could be looking at a lot more activity for writing to the log device (in Sybase ASE it's possible to, effectively, disable logging in tempdb)


• even row-level (exclusive) locks can block other sessions (depends on the isolation level and whether or not a session can scan-past/skip said exclusive locks)

I'd want to go back and (re)investigate the root issue (recompilation locks on the child proc) and see if there's a way to reduce (eliminate?) said compilation locks. [Unfortunately my SQL Server knowledge on these issues is ... NULL ... so would be interested in input from some SQL Server compiler experts.]