Create If Not Exists Transaction Operation Type - #3724
Conversation
There was a problem hiding this comment.
you added a new OperationState.SKIPPED however I think you've missed to update the constraint for tables transactional_model_operation_log, transactional_storage_path_consolidated_operation and transactional_storage_path_operation_log.
Also, since the introduction of flyway in 6.3.0 shouldn't these changes go in a new versioned migration instead of modifying v1? @006627
There was a problem hiding this comment.
@eduardojst10 Yes, since Flyway was introduced in 6.3.0, these changes should go into a new versioned migration rather than modifying V1.
There was a problem hiding this comment.
Latest commit moves and adds schema changes in the new V3 flyway schema migration
006627
left a comment
There was a problem hiding this comment.
A potential rollback issue has already been reported internally. Due to a race condition, a failed transaction could roll back an agent created by another transaction.
For now, the proposal is to never roll back OPTIMISTIC_CREATE_IF_NOT_EXISTS, treating it like READ in rollback().
The reason is that a PREMIS agent is not tied to a specific transaction, so rolling it back could remove an agent created or used by another transaction.
This is a trade-off, as agents created by a failed transaction would remain instead of being removed.
2874a30 to
9caf957
Compare
9caf957 to
bd1a0e5
Compare
As of the current commit, optimistic creates are no longer rolled back in storage, same as with delete operations. |
|
I tried reproducing the scenario this PR is meant to fix, several transactions racing to create the same preservation agent, and hit an edge case worth looking at together before we merge. Scenario: standalone docker, ingested 105 SIPs with the orchestrator tuned to actually force parallel transactions instead of processing them one after another:
What happened: no lock contention at all, saw several transactions hit OPTIMISTIC_CREATE_IF_NOT_EXISTS on the same agent in the same millisecond with no blocking. But 2 out of 105 transactions ended up failing and rolling back, both while racing to create a brand-new agent for the first time. Those two SIPs never made it into storage. 2026-09-08 14:42:50,058 [JobsSystem-io-2-dispatcher-181] ERROR o.r.c.p.o.pekko.PekkoWorkerActor - Error executing plugin.execute()
org.roda.core.plugins.PluginException: Failed to commit transaction for plugin execution, transaction was rolled back
at org.roda.core.transaction.RODATransactionManager.runPluginInTransaction(RODATransactionManager.java:132)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(Unknown Source)
at java.base/java.lang.reflect.Method.invoke(Unknown Source)
at org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:359)
at org.springframework.aop.framework.ReflectiveMethodInvocation.invokeJoinpoint(ReflectiveMethodInvocation.java:190)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:158)
at org.springframework.aop.aspectj.MethodInvocationProceedingJoinPoint.proceed(MethodInvocationProceedingJoinPoint.java:82)
at io.micrometer.core.aop.TimedAspect.processWithTimer(TimedAspect.java:256)
at io.micrometer.core.aop.TimedAspect.perform(TimedAspect.java:222)
at io.micrometer.core.aop.TimedAspect.ajc$inlineAccessMethod$io_micrometer_core_aop_TimedAspect$io_micrometer_core_aop_TimedAspect$perform(TimedAspect.java:1)
at io.micrometer.core.aop.TimedAspect.timedClass(TimedAspect.java:198)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(Unknown Source)
at java.base/java.lang.reflect.Method.invoke(Unknown Source)
at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethodWithGivenArgs(AbstractAspectJAdvice.java:648)
at org.springframework.aop.aspectj.AbstractAspectJAdvice.invokeAdviceMethod(AbstractAspectJAdvice.java:630)
at org.springframework.aop.aspectj.AspectJAroundAdvice.invoke(AspectJAroundAdvice.java:70)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:168)
at org.springframework.aop.interceptor.ExposeInvocationInterceptor.invoke(ExposeInvocationInterceptor.java:96)
at org.springframework.aop.framework.ReflectiveMethodInvocation.proceed(ReflectiveMethodInvocation.java:179)
at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:719)
at org.roda.core.transaction.RODATransactionManager$$SpringCGLIB$$0.runPluginInTransaction(<generated>)
at org.roda.core.plugins.orchestrate.pekko.PekkoWorkerActor.handlePluginExecuteIsReady(PekkoWorkerActor.java:76)
at org.roda.core.plugins.orchestrate.pekko.PekkoWorkerActor.onReceive(PekkoWorkerActor.java:57)
at org.apache.pekko.actor.UntypedAbstractActor$$anonfun$receive$1.applyOrElse(AbstractActor.scala:341)
at org.apache.pekko.actor.Actor.aroundReceive(Actor.scala:547)
at org.apache.pekko.actor.Actor.aroundReceive$(Actor.scala:545)
at org.apache.pekko.actor.AbstractActor.aroundReceive(AbstractActor.scala:229)
at org.apache.pekko.actor.ActorCell.receiveMessage(ActorCell.scala:590)
at org.apache.pekko.actor.ActorCell.invoke(ActorCell.scala:557)
at org.apache.pekko.dispatch.Mailbox.processMailbox(Mailbox.scala:273)
at org.apache.pekko.dispatch.Mailbox.run(Mailbox.scala:234)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
at java.base/java.lang.Thread.run(Unknown Source)
Caused by: org.roda.core.transaction.RODATransactionException: [transactionId:394194ef-7515-43c7-9d46-e9d24cf2cfb5] Failed to copy storage path from staging to main storage service: preservation/agents/urn:roda:premis:agent:org.roda.core.plugins.base.ingest.EARKSIP2ToA
IPPlugin@1.0.xml
at org.roda.core.storage.DefaultTransactionalStorageService.handleOptimisticCreateIfNotExistsOperation(DefaultTransactionalStorageService.java:877)
at org.roda.core.storage.DefaultTransactionalStorageService.commit(DefaultTransactionalStorageService.java:737)
at org.roda.core.transaction.RODATransactionManager.endTransaction(RODATransactionManager.java:169)
at org.roda.core.transaction.RODATransactionManager.runPluginInTransaction(RODATransactionManager.java:126)
... 33 common frames omitted
Caused by: org.roda.core.data.exceptions.GenericException: Could not create binary
at org.roda.core.storage.fs.FileStorageService.createBinary(FileStorageService.java:400)
at org.roda.core.storage.StorageServiceWrapper.createBinary(StorageServiceWrapper.java:131)
at org.roda.core.storage.StorageServiceUtils.copyOrMoveBetweenStorageServices(StorageServiceUtils.java:152)
at org.roda.core.storage.StorageServiceUtils.copyBetweenStorageServices(StorageServiceUtils.java:118)
at org.roda.core.storage.DefaultTransactionalStorageService.handleOptimisticCreateIfNotExistsOperation(DefaultTransactionalStorageService.java:869)
... 36 common frames omitted
Caused by: java.nio.file.FileSystemException: /roda/data/storage/preservation/agents/urn:roda:premis:agent:org.roda.core.plugins.base.ingest.EARKSIP2ToAIPPlugin@1.0.xml
at java.base/sun.nio.fs.UnixFileSystem.copyFile(Unknown Source)
at java.base/sun.nio.fs.UnixFileSystem.copy(Unknown Source)
at java.base/sun.nio.fs.UnixFileSystemProvider.copy(Unknown Source)
at java.base/java.nio.file.Files.copy(Unknown Source)
at org.roda.core.storage.fs.FSPathContentPayload.writeToPath(FSPathContentPayload.java:42)
at org.roda.core.storage.fs.FileStorageService.createBinary(FileStorageService.java:390)
... 40 common frames omittedThis shows up during commit, when the transaction copies the staged agent file over to main storage. It doesn't happen on every run, most concurrent commits go through fine, so it took a few passes of the same test before it actually caught it happening. When it does happen, it's because that staging-to-main copy works as a "replace if exists" write rather than a true atomic create, so under real concurrency it can hit a filesystem error that doesn't get recognized as "already exists" like it's supposed to, and the whole transaction fails instead of just skipping the redundant write. |
e410b73 to
2f46d24
Compare
No description provided.