0

I'm currently working in a legacy application that uses an Stored Procedure in a Sybase DB to generate some IDs incrementally. The procedure is defined as:

CREATE PROC getId
(@val int = -1 output) 
AS
BEGIN
UPDATE ID_TABLE SET LAST_VALUE = LAST_VALUE + 1
SELECT @val = LAST_VALUE FROM ID_TABLE
RETURN @val
END

This procedure is being called from a Java application using Spring's TransactionTemplate to handle the transaction in a declarative manner.

public Integer getId() {
    TransactionTemplate txTemplate = new TransactionTemplate(txManager); // txManager is an autowired instance of PlatformTransactionManager.
    txTemplate.setIsolationLevel(TransactionDefinition.ISOLATION_SERIALIZABLE);
    txTemplate.setTimeout(-1);
    return (Integer) txTemplate.execute((TransactionCallback) status -> idDao.generateId());
}

Internally, idDao uses a JdbcTemplate to call the Stored Procedure using a CallableStatementCreatorFactory. Nothing too out of the ordinary there.

The Stored Procedure is called ~10k times/day. From time to time, we see some ID collisions. My understanding was that the isolation level being set to SERIALIZABLE should prevent this from happening, and I can't seem to reproduce this even calling getId simultaneously with several threads. Does anybody have a hint on what might be happening here?

gnzlrm
  • 190
  • 1
  • 12

1 Answers1

1

I can't speak to the java/spring coding but on a purely SQL basis it looks and sounds like the update/select is not being performed within a transaction.

Without a transaction wrapper (eg, explicit begin/commit tran, autocommit=false, set chained on) the update/select is a prime target for a race condition and duplicate key errors.

NOTE: the isolation level has no effect if the update/select is performed outside of a transaction wrapper

Tracking down a 'missing transaction wrapper' issue is a good idea as there could be other pieces of SQL code suffering from poor/missing transaction management. Having said that ...

For this particular case one easy fix would be to add a transaction wrapper in the proc, eg:

CREATE PROC getId
(@val int = -1 output) 
AS
BEGIN

begin tran
    UPDATE ID_TABLE SET LAST_VALUE = LAST_VALUE + 1
    SELECT @val = LAST_VALUE FROM ID_TABLE
commit tran

RETURN @val
END

Even better would be a rewrite of the update that eliminates the need for the select, eg:

CREATE PROC getId
(@val int = -1 output) 
AS
BEGIN

UPDATE ID_TABLE SET @val=LAST_VALUE+1, LAST_VALUE = LAST_VALUE + 1

RETURN @val
END

NOTE: as a standalone operation this eliminates the need for a transaction wrapper and should eliminate the generation of duplicate keys (assuming this proc is the root cause of the duplicate key issue)

Assuming this is Sybase ASE I'd want to look at making sure the table is configured to use datarows locking; with allpages locking, and to a limited extent datapages locking, there's a chance of a race condition on index updates (which in turn could lead to a deadlock scenario).

markp-fuso
  • 28,790
  • 4
  • 16
  • 36