PHP SilverStripe ORM: Duplicate key value violates unique constraint for DataObject write

Question

I have a function on my website that saves a bunch of values quite quickly to the same DataObject type. Most of the time it's OK but occasionally I get an error

ERROR: duplicate key value violates unique constraint ...

Reading through the documentation I see:

SilverStripe does not use the database's built-in auto-numbering system. Instead, it will generate a new ID by adding 1 to the current maximum ID

And previously looking through the code it looks like it retrieves the max number from the primary key, inserts a record with that ID, then sets the values of the DataObject and writes again. In my load balanced environment, when these multiple entries are sent, I believe the insert is happening with the same primary key, hence the error.

As far as I can see this is an issue I can't get around. From other questions and doco I can't set a composite primary key. Only thing I can think of is to run a custom sql for the create which does use the DB's inbuilt auto-numbering system.

Is there a better way to deal with this error or a way I can set a composite primary key?

EDIT

The full error is

Query failed: ERROR: duplicate key value violates unique constraint 'TABLE_pkey'
DETAIL: Key ('ID')=(136) already exists.

And the statement:

INSERT INTO "TABLE" ("ClassName", "Name", "MemberID", "OtherTabeID", "Value", "LastEdited", "Created", "ID") VALUES ($1, $2, $3, $4, $5, $6, $7, $8),Array) 

I read this as it's inserting the ID from a previously determined value rather than relying on the DB auto-increment. Is that correct?

EDIT 2

Looking through logs it looks like the INSERT is done first with Created field, then select statement is done to get the ID:

SELECT last_value FROM "TABLENAME_ID_seq"

then an UPDATE is done with the additional details being saved.

I feel like this could be a race condition that would cause saving to incorrect rows, though not cause what I'm currently experiencing. Ideally any INSERT would have a returning "ID" that would be used for the update command.

EDIT 3

The above process is contrary to the stack trace I have which shows the insert includes more than just Created:

pg_query_params(Resource id #154,INSERT INTO "TABLENAME" ("ClassName", "Name", "MemberID", "OTHERTABLEID", "Value", "LastEdited", "Created", "ID") VALUES ($1, $2, $3, $4, $5, $6, $7, $8),Array) 
PostgreSQLConnector.php:200
PostgreSQLConnector->preparedQuery(INSERT INTO "TABLENAME" ("ClassName", "Name", "MemberID", "OTHERTABLEID", "Value", "LastEdited", "Created", "ID") VALUES (?, ?, ?, ?, ?, ?, ?, ?),Array,256) 
Database.php:143
SS_Database->{closure}(INSERT INTO "TABLENAME" ("ClassName", "Name", "MemberID", "OTHERTABLEID", "Value", "LastEdited", "Created", "ID") VALUES (?, ?, ?, ?, ?, ?, ?, ?)) 
Database.php:193
SS_Database->benchmarkQuery(INSERT INTO "TABLENAME" ("ClassName", "Name", "MemberID", "OTHERTABLEID", "Value", "LastEdited", "Created", "ID") VALUES (?, ?, ?, ?, ?, ?, ?, ?),Closure,Array) 
Database.php:146
SS_Database->preparedQuery(INSERT INTO "TABLENAME" ("ClassName", "Name", "MemberID", "OTHERTABLEID", "Value", "LastEdited", "Created", "ID") VALUES (?, ?, ?, ?, ?, ?, ?, ?),Array,256) 
DB.php:365
DB::prepared_query(INSERT INTO "TABLENAME" ("ClassName", "Name", "MemberID", "OTHERTABLEID", "Value", "LastEdited", "Created", "ID") VALUES (?, ?, ?, ?, ?, ?, ?, ?),Array) 
SQLExpression.php:121

Answer 1

That note in the documentation is very out of date (even SilverStripe 2.1 from 2007 has the correct behaviour) and the approach described by the docs would lead to race conditions.

The complexity is that SilverStripe uses multi-table inheritance and what SilverStripe does is such cases is this:

• Insert into the SiteTree table
• Fetch the ID generated
• Insert into the Page table (and other tables) using the same ID

It may also do subsequent UPDATE writes to the SiteTree table with the same ID.

Unfortunately this doesn't necessarily help you solve your issue, but it can at least close off one possible source of the issue.

Answer 2

Edit: The documented SilverStripe's generated key support is broken, and uses a method of identifier generation that won't work properly. However, one of the devs has confirmed that this is a documentation bug, and the framework's true behaviour is no longer to use max() queries. So the problem is not there.

For anyone wondering why it's wrong to use max(...) for key generation: it's totally concurrency unsafe. Even in subqueries. If you do:

INSERT INTO my_table(id, ...)
VALUES
(
  (SELECT max(id) + 1 FROM my_table),
  ...
);

then both SELECTs can run at the same time. They'll get the same result, and then both inserts will try to insert the same value. Even if one insert completes before the other select runs, if it hasn't committed yet, the other select won't see the new value.

It's only safe to do that if you LOCK TABLE first, or SELECT ... FOR UPDATE in the subquery. A SELECT ... FOR UPDATE is much slower in this case.

So if you can modify SilverStripe, change it to at least send a LOCK TABLE mytable IN EXCLUSIVE MODE; before the SELECT max(...) so it's slow and clumsy but not also broken.

Or, better, fix it to just use a database sequence.

If there's a real business need for gap-less numbering, use a counter table maintained by UPDATE ... RETURNING ... instead. (If you need portability you must use SELECT ... FOR UPDATE then UPDATE, in the same transaction).

Update: The framework no longer uses that approach in recent versions.

(Removed grumpy rant about the approach as non-constructive)

Answer 3

As @CraigRinger pointed out, the way the inserted ID was being accessed was across all sessions, rather than per session. I've updated the PostgreSQL module to instead use currval() which is session based.

So far I haven't replicated the issue again however I'm not entirely convinced this is the core of the issue. I'll update this if it faults again.