Entity creation rejected with unrelated transaction error

[pingtimeout]

Describe the bug

Under a low concurrency test, entity updates are rejected with an HTTP 500 error. This applies for catalog updates, namespace creation, table updates, ...

To Reproduce

• Check out this commit: pingtimeout@fbd3b90
• Run the server using the getting-started docker compose file: docker compose -f getting-started/eclipselink/docker-compose.yml up
• Export the client ID and secrets as environment variables: export CLIENT_ID=root CLIENT_SECRET=s3cr3t
• Run ./gradlew :polaris-benchmarks:gatlingRun

The simulation will run 5 concurrent users. Each user creates its own catalog (named C_0, C_1, ...). Then, each user sequentially creates 5 namespaces under its own catalog (named NS_0, NS_1, ...).

Actual Behavior

The Gatling output consistently shows that not all catalogs nor all namespaces could be created. In the output below, only 1 catalog was created and the other 4 creations were rejected with an HTTP 500 error.

========================================================================================================================
2025-03-05 16:42:50 UTC                                                                               0s elapsed
---- Requests -----------------------------------------------------------------------|---Total---|-----OK----|----KO----
> Global                                                                             |        35 |        11 |        24
> Authenticate                                                                       |         5 |         5 |         0
> Create Catalog                                                                     |         5 |         1 |         4
> Create Namespace                                                                   |        25 |         5 |        20
---- Errors ------------------------------------------------------------------------------------------------------------
> status.find.is(200), but actually found 404                                                                20 (83.33%)
> status.find.is(201), but actually found 500                                                                 4 (16.67%)

This file is the server log for the Polaris instance. It contains numerous errors like the one below

2025-03-05 16:35:20 INFO  [org.apache.polaris.service.exception.IcebergExceptionMapper] (executor-thread-1) Handling runtimeException Exception [EclipseLink-4002] (Eclipse Persistence Services - 4.0.5.v202412231137-a96b873527f305f932543045c8679bb1de8d3a43): org.eclipse.persistence.exceptions.DatabaseException
Internal Exception: org.postgresql.util.PSQLException: ERROR: could not serialize access due to read/write dependencies among transactions
  Detail: Reason code: Canceled on identification as a pivot, during conflict out checking.
  Hint: The transaction might succeed if retried.
Error Code: 0
Call: UPDATE ENTITIES SET GRANTRECORDSVERSION = ?, VERSION = ? WHERE (((CATALOGID = ?) AND (ID = ?)) AND (VERSION = ?))
        bind => [5 parameters bound]
Query: UpdateObjectQuery(org.apache.polaris.jpa.models.ModelEntity@2da9cea8)

Those errors are not caught and result in a HTTP 500 response to be sent to the client. Here is the payload that is received on the Gatling side:

{"error":{"message":"Exception [EclipseLink-4002] (Eclipse Persistence Services - 4.0.5.v202412231137-a96b873527f305f932543045c8679bb1de8d3a43): org.eclipse.persistence.exceptions.DatabaseException\nInternal Exception: org.postgresql.util.
PSQLException: ERROR: could not serialize access due to concurrent update\nError Code: 0\nCall: UPDATE ENTITIES SET GRANTRECORDSVERSION = ?, VERSION = ? WHERE (((CATALOGID = ?) AND (ID = ?)) AND (VERSION = ?))\n\tbind => [5 parameters boun
d]\nQuery: UpdateObjectQuery(org.apache.polaris.jpa.models.ModelEntity@1123186d)","type":"PersistenceException","code":500}}

Expected Behavior

Given that there is no overlap between catalogs and namespaces, all queries should succeed.

Additional context

This result was reproduced even after #1092 has been merged on main.

System information

No response

[flyrain]

Thanks @pingtimeout for reporting it. We are actively working on it. cc @singhpk234 @dennishuo @jbonofre

[pingtimeout]

Possibly related to #1202

[dennishuo]

The AtomicOperation flavor that @singhpk234 is working on should definitely eliminate a lot of the contention problems, but I believe the main problem with the EclipseLink impl right now is that the name portion of the entities_active table isn't included as part of the primary key. And for some reason id is. Basically ModelEntityActive is completely wrong.

/**
 * EntityActive model representing some attributes of a Polaris Entity. This is used to exchange
 * entity information with ENTITIES_ACTIVE table
 */
@Entity
@Table(name = "ENTITIES_ACTIVE")
public class ModelEntityActive {
  // entity catalog id
  @Id private long catalogId;

  // id of the entity
  @Id private long id;

  // parent id of the entity
  @Id private long parentId;

  // name of the entity
  private String name;

  // code representing the type of that entity
  @Id private int typeCode;

  // code representing the subtype of that entity
  private int subTypeCode;

If I run the docker image from "getting-started" and shell into the postgres container to inspect the table schemas, we can see that the annotations indeed construct primary keys as we'd expect, just that we annotated the wrong fields to be the primary key:

root@fe996aabfb1b:/# psql -U postgres -d POLARIS
psql (17.4 (Debian 17.4-1.pgdg120+2))
Type "help" for help.

POLARIS=# select * from pg_catalog.pg_tables;
POLARIS=# \d entities;
                            Table "public.entities"
       Column        |          Type          | Collation | Nullable | Default 
---------------------+------------------------+-----------+----------+---------
 catalogid           | bigint                 |           | not null | 
 id                  | bigint                 |           | not null | 
 createtimestamp     | bigint                 |           |          | 
 droptimestamp       | bigint                 |           |          | 
 entityversion       | integer                |           |          | 
 grantrecordsversion | integer                |           |          | 
 internalproperties  | text                   |           |          | 
 lastupdatetimestamp | bigint                 |           |          | 
 name                | character varying(255) |           |          | 
 parentid            | bigint                 |           |          | 
 properties          | text                   |           |          | 
 purgetimestamp      | bigint                 |           |          | 
 subtypecode         | integer                |           |          | 
 topurgetimestamp    | bigint                 |           |          | 
 typecode            | integer                |           |          | 
 version             | bigint                 |           |          | 
Indexes:
    "entities_pkey" PRIMARY KEY, btree (catalogid, id)

POLARIS=# \d entities_active;
                    Table "public.entities_active"
   Column    |          Type          | Collation | Nullable | Default 
-------------+------------------------+-----------+----------+---------
 catalogid   | bigint                 |           | not null | 
 id          | bigint                 |           | not null | 
 parentid    | bigint                 |           | not null | 
 typecode    | integer                |           | not null | 
 name        | character varying(255) |           |          | 
 subtypecode | integer                |           |          | 
Indexes:
    "entities_active_pkey" PRIMARY KEY, btree (catalogid, id, parentid, typecode)

POLARIS=# \d entities_change_tracking;
            Table "public.entities_change_tracking"
       Column        |  Type   | Collation | Nullable | Default 
---------------------+---------+-----------+----------+---------
 catalogid           | bigint  |           | not null | 
 id                  | bigint  |           | not null | 
 entityversion       | integer |           |          | 
 grantrecordsversion | integer |           |          | 
 version             | bigint  |           |          | 
Indexes:
    "entities_change_tracking_pkey" PRIMARY KEY, btree (catalogid, id)

We really need entities_active_pkey to be (catalogid, parentid, typecode, name) instead of (catalogid, id, parentid, typecode).

Without the index that includes the name, every transaction that includes a lookupByName would effectively require a lock on the entire catalogid, parentid, typecode for needless contention within the same "container", but with the current catalogid, id, parentid, typecode PK it's actually the worst-case because lookupByName pollutes the transaction with the entire catalogid as the only prefix portion of the PK for query patterns that use the ENTITIES_ACTIVE table at all.

So all root-level entities conflict with each other (PRINCIPALS, CATALOGS, PRINCIPAL_ROLES), and all catalog objects conflict with each other in a catalog (TABLE/VIEW, CATALOG_ROLE, NAMESPACE).

Unfortunately I'm not sure how EclipseLink+Postgres behaves if we want to change the primary-key in-place.

In theory, according to docs I've read, if we simply add the desired UNIQUE INDEX on (catalogid, parentid, typecode, name) then Postgres will do the right thing and single-name lookups should no longer conflict with unrelated changes, even if we keep using this three-table EclipseLink implementation.

[dennishuo]

Note that in this case CreateCatalog specifically might still have contention on the service_admin PrincipalRole because it has to assign new CatalogRoles to the same PrincipalRole by default (though if we make all 5 users use different PrincipalRoles and I can't remember if the logic correctly finds the non-service_admin PrincipalRole to assign the catalog_admin to in that case) then the PrincipalRole contention should also be resolved.

[dennishuo]

Indeed if I fix the ModelEntitiesActive index, I still hit concurrency issues at the catalog level and I see these:

polaris-1            | Caused by: Exception [EclipseLink-5006] (Eclipse Persistence Services - 4.0.5.v202412231137-a96b873527f305f932543045c8679bb1de8d3a43): org.eclipse.persistence.exceptions.OptimisticLockException
polaris-1            | Exception Description: The object [org.apache.polaris.jpa.models.ModelEntity@52bad184] cannot be updated because it has changed or been deleted since it was last read. 
polaris-1            | Class> org.apache.polaris.jpa.models.ModelEntity Primary Key> [[0, 2]: 963]

If I lookup that primary key then sure enough the problem is the service_admin:

POLARIS=# select catalogid, id, entityversion, grantrecordsversion, name from entities where catalogid = 0 and id = 2;
 catalogid | id | entityversion | grantrecordsversion |     name      
-----------+----+---------------+---------------------+---------------
         0 |  2 |             1 |                   8 | service_admin
(1 row)

[dennishuo]

Playing around with it more, unfortunately it does seem like there are still transaction serialization conflicts during the inserts even after fixing the entities_active index. We should probably still fix the index if possible since otherwise lookupByName right now is horribly inefficient, but ultimately for the legacy three-entity-table EclipseLink implementation we probably do need to add low-level retries at least, and the new AtomicOperation postgres impl should fare much better. We'll still want to add low-level retries where appropriate in AtomicOperationMetaStoreManager though.

[singhpk234]

I completed the draft impl, will start sending small prs soon

I ran the tests with the AtomicMetaStoreManager which doesn't required transactions, now this passes 100%

➜  polaris git:(transaction-dependencies-issue) ✗ ./gradlew :polaris-benchmarks:gatlingRun
Configuration on demand is an incubating feature.

> Task :polaris-build-logic:compileKotlin UP-TO-DATE
Kotlin does not yet support 23 JDK target, falling back to Kotlin JVM_22 JVM target

> Configure project :
Kotlin does not yet support 23 JDK target, falling back to Kotlin JVM_22 JVM target

> Task :polaris-benchmarks:gatlingRun
Gatling 3.13.5 is available! (you're using 3.13.4)
Simulation IcebergRestSimulation started...

========================================================================================================================
2025-03-27 01:00:15 GMT                                                                               0s elapsed
---- Requests -----------------------------------------------------------------------|---Total---|-----OK----|----KO----
> Global                                                                             |        35 |        35 |         0
> Authenticate                                                                       |         5 |         5 |         0
> Create Catalog                                                                     |         5 |         5 |         0
> Create Namespace                                                                   |        25 |        25 |         0

---- Create catalog and namespaces using the Iceberg REST API ----------------------------------------------------------
[################################################################################################################]  100%
          waiting:         0 / active:         0  / done:         5
========================================================================================================================

Parsing log file(s)...
Parsing log file(s) done in 0s.
Generating reports...

========================================================================================================================
---- Global Information -------------------------------------------------------------|---Total---|-----OK----|----KO----
> request count                                                                      |        35 |        35 |         0
> min response time (ms)                                                             |        37 |        37 |         -
> max response time (ms)                                                             |        76 |        76 |         -
> mean response time (ms)                                                            |        54 |        54 |         -
> response time std deviation (ms)                                                   |        10 |        10 |         -
> response time 50th percentile (ms)                                                 |        56 |        56 |         -
> response time 75th percentile (ms)                                                 |        60 |        60 |         -
> response time 95th percentile (ms)                                                 |        68 |        68 |         -
> response time 99th percentile (ms)                                                 |        76 |        76 |         -
> mean throughput (rps)                                                              |        35 |        35 |         -
---- Response Time Distribution ----------------------------------------------------------------------------------------
> OK: t < 800 ms                                                                                             35   (100%)
> OK: 800 ms <= t < 1200 ms                                                                                   0     (0%)
> OK: t >= 1200 ms                                                                                            0     (0%)
> KO                                                                                                          0     (0%)
========================================================================================================================

Reports generated, please open the following file: file:///Users/prsingh/Desktop/oss-work/polaris/benchmarks/build/reports/gatling/icebergrestsimulation-20250327010014436/index.html

[Incubating] Problems report is available at: file:///Users/prsingh/Desktop/oss-work/polaris/build/reports/problems/problems-report.html

Deprecated Gradle features were used in this build, making it incompatible with Gradle 9.0.

You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins.

For more on this, please refer to https://docs.gradle.org/8.12.1/userguide/command_line_interface.html#sec:command_line_warnings in the Gradle documentation.

[flyrain]

Thanks @singhpk234! This is a good starting point! I assume this is the test with 5 concurrent users. Wondering perf under bigger concurrent number like 50.

We may start to setup perf pipeline to test against the main branch periodically.

[flyrain]

Close this as the JDBC implementation was done.