Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Batch Request does not take dependsOn into account #762

Open
MartinTopfstedtSinc opened this issue Jul 18, 2024 · 3 comments
Open

Batch Request does not take dependsOn into account #762

MartinTopfstedtSinc opened this issue Jul 18, 2024 · 3 comments

Comments

@MartinTopfstedtSinc
Copy link

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

await Task.WhenAll(responseTasks).ConfigureAwait(false);

@MitchKuijpers
Copy link
Contributor

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

@jspuij
Copy link
Contributor

jspuij commented Aug 19, 2024

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?

@robertmclaws
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants