Multi-instance query support #32
Conversation
|
Looks like there's an issue with the CI - the sidecar doesn't appear to be running correctly. We should look into introducing shorter timeouts to the integration tests so that we don't have to wait a full hour for the CI to fail. |
sorry that I forget to set the default timeout back after local developing, will set it back soon in next commit. |
|
once reset resources logics is there, the pipeline should success. |
|
I believe once the sidecar image is updated the pipeline should success, as it's what I saw on my local. |
|
Okay, let me update the sidecar image and retry the CI. |
|
CI is green after updating the sidecar image 👍🏽 |
sdk/src/main/java/com/microsoft/durabletask/OrchestrationStatusQuery.java
Show resolved
Hide resolved
sdk/src/main/java/com/microsoft/durabletask/OrchestrationStatusQuery.java
Show resolved
Hide resolved
sdk/src/main/java/com/microsoft/durabletask/OrchestrationStatusQueryResult.java
Show resolved
Hide resolved
|
|
||
| public class OrchestrationStatusQueryResult { | ||
| public List<OrchestrationMetadata> OrchestrationState; | ||
| public String continuationToken; |
There was a problem hiding this comment.
Let's remove the setters, make the fields private readonly, and have them populated using a constructor. That way instances of this class can be immutable, which is a useful property for ensuring code correctness.
|
|
||
| for (int i = 0; i < 5; i++){ | ||
| final int j = i; | ||
| CompletableFuture<OrchestrationMetadata> waitEvent = CompletableFuture.supplyAsync(() -> this.startOrchestrationAsync(waitForEvent, String.valueOf(j), new StringBuilder(prefix).append(".waiter.").append(j).toString(), client)); |
There was a problem hiding this comment.
I understand how you might want to use CompletableFuture here to make the code similar to the C# version. However, C# has async/await, which makes it very easy to do non-blocking code in parallel. Java doesn't have this and CompletableFuture, unfortunately, is not always a good substitute. However, we can still express the same idea in a way that is actually quite simple and easy to understand.
Here is an example that I came up with. It's fewer lines of code and still executes everything in parallel. I think this version makes the code much more readable and maintainable:
// Run 5 orchestrations in parallel and wait for them all to finish
IntStream.range(0, 5).mapToObj(i -> {
String instanceId = String.format("%s.waiter.%d", prefix, i);
client.scheduleNewOrchestrationInstance(waitForEvent, String.valueOf(i), instanceId);
return instanceId;
}).collect(Collectors.toUnmodifiableList()).forEach(id -> {
client.waitForInstanceStart(id, defaultTimeout);
});Note that I've also removed the helper methods, so I'm only using public APIs. It's nice if we can try to do this because it makes it easy to copy/paste the code to share with someone as a sample for how to do something similar. Adding too many helper methods like runOrchestrationAsync and startOrchestrationAsync makes this much more difficult. The C# code you looked at uses helper methods because the public APIs are not as cleanly designed as these Java APIs and don't support things like lambda expressions.
| String name = ctx.getInput(String.class); | ||
| String output = ctx.waitForExternalEvent(name, String.class).get(); | ||
| ctx.complete(output); | ||
| }); |
There was a problem hiding this comment.
Is it really beneficial to save these orchestrator functions into a static map? If it's not too much ceremony, I would prefer we follow the existing code pattern and define the orchestrations and activities directly in the test method.
This PR implements logics for supporting multi-instance query (#9)
This PR includes: