vlingo / xoom-common

The VLINGO XOOM platform SDK common tools shared across various projects.
Other
16 stars 10 forks source link

Redesign Completes<T> so it decouples composition and execution time #16

Open kmruiz opened 5 years ago

kmruiz commented 5 years ago

We realised after a long conversation that Completes<T> right now has an important issue that needs to be tackled: a Completes<T> pipeline can be run during the composition fase, because it runs potentially in different threads.

Our new proposal is to actually rework how Completes<T> works so it can be simplified, more efficient and easier to change. The design decisions that we want to take:

Opinions and feedback are really appreciated.

It's also a good opportunity to work on the following issues and solve them:

kmruiz commented 5 years ago

@VaughnVernon I was thinking on composing operations, like we did on this closed PR that didn't work (because of multithreading): https://github.com/vlingo/vlingo-common/pull/11

kmruiz commented 5 years ago

Right now the new implementation requires the java property vlingo.InMemoryCompletes set to true to run. Any other value on this property (or keeping it empty) will run with BasicCompletes

tjaskula commented 5 years ago

@kmruiz @VaughnVernon is the new implementation working on Java or there are more tweaks to come ?

VaughnVernon commented 5 years ago

@tjaskula I think that the design itself is golden. The use of ready() greatly simplifies the overall design because there can be a single thread assigned to execution. It is possible that there will be some tweaks but I think the current design reflected here will remain:

https://github.com/vlingo/vlingo-common/tree/master/src/main/java/io/vlingo/common/completes

Other than the return type parameter for .NET you should be able to use this to great advantage :)

VaughnVernon commented 5 years ago

@kmruiz I have added andFinally() and andFinally(function). The knowhow is in the second overload. I assume I have implemented it correctly per the andFinallyConsume(consumer) implementation. I have tested this locally and it works fine. However, on vlingo-actors it hangs in the StageTest when querying the Directory for given actors by address. I think that this implementation is subject to predictive execution race conditions where countdowns are seen in dependent threads before the outcome state is set. The behavior I see is pretty typical of these kinds of inconsistencies. Although I much prefer the simplicity of this implementation is seems to require a considerable amount of additional testing to get it across the finish line. Unfortunately it will probably tend to fail on CI and machines other than the developer's.