Introduce entity work items and connect them to ITaskEntity definition.

[sebastianburckhardt]

Defines entity work items as a third work item type, and implements the necessary shim classes to connect it to ITaskEntity and the user-facing entity SDK.

[jviau]

Adding this is a breaking change. Easiest fix may be to add a new interface IDurableTaskFactory2 and add this there. Then consumers will need to cast to see if it implements the 2nd interface.

We can then consolidate down to single interface on next major version rev. Or even come up with a way to add new work item types without breaking changes to this interface.

[sebastianburckhardt]

I have added an IDurableTaskFactory2.

Note that I haven't actually made the necessary changes to truly create the entities from whatever way they are annotated in the code. That is one of the open work items.

[jviau]

I would feel slightly better if the operation where immutable and it was new'd up each time. This is one of the reasons I split out the TaskEntityContext into its own object, so that it didn't need to new new'd up.

[sebastianburckhardt]

This was a purposeful optimization to reduce allocations. From a code elegance perspective, using a new would obviously be nicer but I think we should try to execute the inner loop (operations in a batch) as efficiently as possible.

[jviau]

ctor can fit on one line I think (I keep to 120 col limit)

[jviau]

It took me a bit to read and understand the commit/rollback logic here. At first I was surprised it was needed at all, but I get it now. Two suggestions for this:

1. Add a lengthy comment somewhere explaining what it is and why it is needed. (IE: on exception we need to drop any performed actions and state changes).
2. Code organization, consider the following:
   1. We want to roll back state & context, so lets add the rollback logic directly into those.
   2. The outer types Commit() and Rollback() methods simply call context.Commit(); state.Commit(); and context.Rollback(); state.Rollback(); respectively. No need for the outer class to understand what the exact commit/rollback logic is.

[sebastianburckhardt]

FYI, this behavior is controllable via a parameter in DF (i.e. the rollback behavior can be turned off). I am wondering if we should try to honor this parameter here also. It adds complexity though so I would rather not (I am not sure how to even implement configuration settings).

This was the original discussion that led us to implement the rollback: Azure/azure-functions-durable-extension#1219

[jviau]

If you go with the commit/rollback suggestion of mine, can make this a Reset().

[sebastianburckhardt]

I have revised the Commit/Rollback/Reset.

[jviau]

If we want to manage GC, maybe all the shim objects can be created at the start of ExecuteOperationBatchAsync and then let them go out of scope and be GC'd entirely.

I personally don't think optimizing to re-use these objects between batches is worth the added complexity.

[sebastianburckhardt]

I think it is worth it if the batches are large and the operations are very short. For example, 10,000 increment operations on a counter. Reducing allocations is a pretty standard optimization goal for .NET code, and very typical for SDK design.

[sebastianburckhardt]

The nice part here is that the user does not really see these optimizations, from the outside it looks clean.

[jviau]

If you want to keep it the way it is, that is fine. Not my style, but I'll live 😄