Concurrent Namespace Update Fails with RuntimeException and 500 Error

[BenjaminSSL]

Describe the bug

When performing concurrent parallel HTTP requests involving UPDATE operations on namespace properties, an RuntimeException is encountered.

The error message in the Polaris logs indicates a Concurrent modification of namespace issue:

ERROR [org.apa.pol.ser.exc.IcebergExceptionMapper] [,POLARIS] [,,,] (executor-thread-2) Unhandled exception returning INTERNAL_SERVER_ERROR: java.lang.RuntimeException: Concurrent modification of namespace: f62452d6-39fc-40a8-87a5-c08f6ee65c80
	at org.apache.polaris.service.catalog.iceberg.IcebergCatalog.setProperties(IcebergCatalog.java:710)
	at org.apache.iceberg.rest.CatalogHandlers.updateNamespaceProperties(CatalogHandlers.java:193)
	at org.apache.polaris.service.catalog.iceberg.IcebergCatalogHandlerWrapper.updateNamespaceProperties(IcebergCatalogHandlerWrapper.java:538)
	at org.apache.polaris.service.catalog.iceberg.IcebergCatalogAdapter.lambda$updateProperties$5(IcebergCatalogAdapter.java:268)
	at org.apache.polaris.service.catalog.iceberg.IcebergCatalogAdapter.withCatalog(IcebergCatalogAdapter.java:161)
	at org.apache.polaris.service.catalog.iceberg.IcebergCatalogAdapter.updateProperties(IcebergCatalogAdapter.java:264)
	at org.apache.polaris.service.catalog.iceberg.IcebergCatalogAdapter_ClientProxy.updateProperties(Unknown Source)
	at org.apache.polaris.service.catalog.api.IcebergRestCatalogApi.updateProperties(IcebergRestCatalogApi.java:911)
	at org.apache.polaris.service.catalog.api.IcebergRestCatalogApi_Subclass.updateProperties$$superforward(Unknown Source)
	at org.apache.polaris.service.catalog.api.IcebergRestCatalogApi_Subclass$$function$$23.apply(Unknown Source)
	at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:73)
	at io.quarkus.arc.impl.AroundInvokeInvocationContext.proceed(AroundInvokeInvocationContext.java:62)
	at io.quarkus.micrometer.runtime.MicrometerTimedInterceptor.timedMethod(MicrometerTimedInterceptor.java:79)
	at io.quarkus.micrometer.runtime.MicrometerTimedInterceptor_Bean.intercept(Unknown Source)
	at io.quarkus.arc.impl.InterceptorInvocation.invoke(InterceptorInvocation.java:42)
	at io.quarkus.arc.impl.AroundInvokeInvocationContext.perform(AroundInvokeInvocationContext.java:30)
	at io.quarkus.arc.impl.InvocationContexts.performAroundInvoke(InvocationContexts.java:27)
	at org.apache.polaris.service.catalog.api.IcebergRestCatalogApi_Subclass.updateProperties(Unknown Source)
	at org.apache.polaris.service.catalog.api.IcebergRestCatalogApi$quarkusrestinvoker$updateProperties_d0c974b892d05b0179f2423c7b38060f4f880bc3.invoke(Unknown Source)
	at org.jboss.resteasy.reactive.server.handlers.InvocationHandler.handle(InvocationHandler.java:29)
	at io.quarkus.resteasy.reactive.server.runtime.QuarkusResteasyReactiveRequestContext.invokeHandler(QuarkusResteasyReactiveRequestContext.java:141)
	at org.jboss.resteasy.reactive.common.core.AbstractResteasyReactiveContext.run(AbstractResteasyReactiveContext.java:147)
	at io.quarkus.vertx.core.runtime.VertxCoreRecorder$15.runWith(VertxCoreRecorder.java:638)
	at org.jboss.threads.EnhancedQueueExecutor$Task.doRunWith(EnhancedQueueExecutor.java:2675)
	at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2654)
	at org.jboss.threads.EnhancedQueueExecutor.runThreadBody(EnhancedQueueExecutor.java:1627)
	at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1594)
	at org.jboss.threads.DelegatingRunnable.run(DelegatingRunnable.java:11)
	at org.jboss.threads.ThreadLocalResettingRunnable.run(ThreadLocalResettingRunnable.java:11)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.base/java.lang.Thread.run(Thread.java:1583)

I’m a bit unsure whether this is actually a bug, but it seems like one. Is it expected that concurrent update requests on the same entity are not supported?

When I tested concurrent requests on catalog, namespace, table, and view entities, I received a CommitFailedException with a 409 status code. I assume that’s the expected behavior. However, I’d like to confirm whether concurrent updates on the same entity (in this case, a namespace) should be allowed or not.

To Reproduce

Setup a test with a minimum of two threads:

• Ensure that the top-level catalog and a namespace are created before running the test.
• The $n$ amount of threads should UPDATE the properties on the namespace in parallel and repeatedly.

Run the test for at least 10 seconds.

During the test, the UPDATE operation will occasionally fail with a 500 error. However, it does not fail every time or follow a clear pattern.

This is the endpoint I used to update the properties:

http://localhost:8181/api/catalog/v1/polaris/namespaces/test/properties

Actual Behavior

The UPDATE operation on the namespace properties fails with a 500 error. The error message indicates a concurrent modification of namespace issue.

Expected Behavior

If concurrent modifications are not allowed, I would expect a 409 error with a message explaining the conflict. If they are allowed, I’d expect the update to succeed consistently without errors.

Additional context

Test environment used the in-memory store.

System information

OS: MacOS Sonoma v15.4.1
Polaris Catalog Version: 1.0.0-incubating-SNAPSHOT
Object Storage: FILE

[MonkeyCanCode]

I think this is expected. For conflict transactions to entities used by Polaris, it will try to rollback (PR ref: https://github.com/apache/polaris/pull/1092/files). @dennishuo can confirm for the above behavior.

[hemartin]

Hi @MonkeyCanCode and @dennishuo. Dennis, you might remember me from my time at Snowflake. Now I'm an associate professor at the IT University of Copenhagen. Together with a student, Benjamin, we have done some concurrency testing of Polaris. We've run into this issue as well as #1388 and #1389.

We wanted to report this behavior here because we think that throwing a RuntimeException to the user is not expected behavior. If concurrent modifications of objects are forbidden or if you roll back internally, perhaps a more descriptive user error should be thrown. Thanks for checking this.

[dimas-b]

I agree that 500 errors are never "expected behaviour" and Polaris should return a more appropriate message and error code in this case.

[dimas-b]

That said, I wonder if EclipseLink was used for Polaris Persistence in this case. If so, I'd recommend re-testing with the new JDBC Persistence impl. as EclipseLink is effectively deprecated. Cf. #1515

[singhpk234]

Adding more details, so essentially the IcebergCatalog at service layer itself thows up code persistence is throwing current concurrent modification exception, but the thing is the service layer is re-throwing it as RTE, which need to handled in the IcebergExceptionMapper.

[dimas-b]

Note: The POST verb on the namespace properties endpoint in the Iceberg REST Catalog spec does not define a 409 response code.

Moreover, NamespaceErrorHandler in the java REST client maps 409 to AlreadyExistsException.

I tend to think a 409 response code would be correct in this case, but the java client may present it in an awkward way to its caller.