RESTier icon indicating copy to clipboard operation
RESTier copied to clipboard

Batch Request does not take dependsOn into account

Open MartinTopfstedtSinc opened this issue 1 year ago • 4 comments

A batch request with 2 request, where the second request depends on the first. The first requests creates a new item and the second should create a new related item to the item from the first request, therefore the "$ContentId" url syntax is used.in the second request. Both requests are in the same atomicityGroup.

The Problem is that both request get executed in parallel, and the second request can' t replace the "$ContentId" in the url because the first request didn't finish yet.

Assemblies affected

Microsoft.Restier.AspNetCore 1.1.1

Reproduce steps

A batch request contains 2 request, where the second request depends on the first. The first requests creates a new item and the second should create a new related item to the item from the first request, therefore the "$1" url syntax is used. Both requests are in the same atomicityGroup. The simplest set of steps to reproduce the issue. If possible, reference a commit that demonstrates the issue.

Expected result

The BatchChangeSetRequestItem should consider the dependsOn property of a request and order them correctly and execute only possible requests in parallel.

Actual result

Both requests are executed in parallel.

Additional details

For a small example see: https://docs.oasis-open.org/odata/odata-json-format/v4.01/odata-json-format-v4.01.html#sec_ReferencingNewEntities

https://github.com/OData/RESTier/blob/013bc0699cbf440dac78c9c07df30e8377d8b631/src/Microsoft.Restier.AspNet/Batch/RestierBatchChangeSetRequestItem.cs#L98

MartinTopfstedtSinc avatar Jul 18 '24 12:07 MartinTopfstedtSinc

I think sending a singlechangset should never be async? However for splitting into operations it should be fine...

MitchKuijpers avatar Aug 12 '24 12:08 MitchKuijpers

This is an interesting issue, because I think it highlights some delicate formulations in the OData spec. (I used the formal OData 4.0 spec to cross reference).

Firstly, requests within a batch have to be processed in order. Requests within a change-set MAY be executed in any order (but it's not mandatory).

https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ProcessingaMultipartBatchRequest

However, within a change-set previously inserted entities CAN be referenced by a content-id:

https://docs.oasis-open.org/odata/odata/v4.01/odata-v4.01-part1-protocol.html#sec_ReferencingNewEntities

This of course implies, as @MartinTopfstedtSinc noted, that any request on a relational database that has foreign key constraints enabled cannot be executed in parallel.

@robertmclaws I think this is something we have to fix. Now there are multiple ways to go about this:

  • Disable parallelisation entirely. This of course is the slowest option.
  • Disable parallelisation of the change-set whenever a dependency is detected. This enables parallelisation of change-sets whenever they contain no dependencies at all.
  • Resolve the dependency graph. This requires building a dependency graph before processing and subsequently solving it before executing the change-set. The neatest, but also the most time-consuming solution.

Any thoughts?

jspuij avatar Aug 19 '24 10:08 jspuij

My plan was to detect if dependencies were involved and if so, disable parallelization.

robertmclaws avatar Aug 19 '24 16:08 robertmclaws

I'm working on getting this library to .NET 9 over the weekend, so I'll take a look at getting a fix for this in one way or another,

robertmclaws avatar Nov 15 '24 17:11 robertmclaws