mgmt resources, sync implementation for TaskGroup #45068

XiaofeiCao · 2025-04-22T09:38:14Z

Description

Part of sync-stack support for mgmt premium, and core-v2 if we want to keep premium packages.

Goal

Provide sync interfaces for TaskGroup invocation without Reactor..
Support specifying Executor to achieve task execution parallelism.

Context

A rough flow of TaskGroup.invokeAsync:

About this PR

TaskGroup.invoke's major change is in step4 -> Concurrently execute tasks.
Instead of Flux.merge, use CompletableFuture.allOf to join all CompletableFutures kicked off by CompletableFuture.supplyAsync with default or specified Executor.

Specify Executor by setting syncExecutor in InvocationContext, and execute via TaskGroup.invoke(InvocationContext).

To run task in the same thread as TaskGroup.invoke, set syncExecutor to Runnable::run.

Default Executor(if not specified) is ForkJoinPool.commonPool(), which is also the default Executor for CompletableFuture.

Test

Added tests in the same package as async ones, with Sync prefix.

Future works

New sync APIs in resource base models, with Reactor as its default implementation.
Override these new sync Resource API implementations for all packages.
Remove Reactor default implementations for sync APIs.

Please add an informative description that covers that changes made by the pull request and link all relevant issues.

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.

All SDK Contribution checklist:

The pull request does not introduce [breaking changes]
CHANGELOG is updated for new features, bug fixes or other significant changes.
I have read the contribution guidelines.

General Guidelines and Best Practices

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

azure-sdk · 2025-04-22T09:58:35Z

API Change Check

APIView identified API level changes in this PR and created the following API reviews

com.azure.resourcemanager:azure-resourcemanager-resources

azure-sdk · 2025-04-24T10:21:00Z

API change check

APIView has identified API level changes in this PR and created following API reviews.

com.azure.resourcemanager:azure-resourcemanager-resources

XiaofeiCao · 2025-04-27T07:07:07Z

.../java/com/azure/resourcemanager/resources/fluentcore/dag/ErroredDependencyTaskException.java

 */
-final class ErroredDependencyTaskException extends Exception {
+final class ErroredDependencyTaskException extends RuntimeException {


It's not expected for user to catch this Exception. It'll be caught only in TaskGroup implementation.

maybe a slightly easy-to-read name, errored feels strange? FailedDependencyTaskException?

XiaofeiCao · 2025-04-27T08:57:18Z

Not sure why pipeline revapi keeps complaining about breaking change of TaskGroup.failedFuture from 2.50.0.

This method wasn't even added in 2.50.0.

My local build and check doesn't have this issue...

weidongxu-microsoft

I did a quick read and didn't see problem.

However, merging of the PR would mean that it be under revapi check.

PS: also not sure why failedFuture errors (and the old Analysis CI passes).

weidongxu-microsoft · 2025-04-28T02:16:52Z

.../java/com/azure/resourcemanager/resources/fluentcore/dag/ErroredDependencyTaskException.java

 */
-final class ErroredDependencyTaskException extends Exception {
+final class ErroredDependencyTaskException extends RuntimeException {


maybe a slightly easy-to-read name, errored feels strange? FailedDependencyTaskException?

...er-resources/src/main/java/com/azure/resourcemanager/resources/fluentcore/dag/TaskGroup.java

weidongxu-microsoft · 2025-04-28T02:42:17Z

...er-resources/src/main/java/com/azure/resourcemanager/resources/fluentcore/dag/TaskGroup.java

+        public InvocationContext withSyncTaskExecutor(Executor syncExecutor) {
+            Objects.requireNonNull(syncExecutor);
+            this.syncExecutor = syncExecutor;
+            return this;
+        }


Is this supposed to be called by our lib in its API (when create the TaskGroup), or by user? Likely won't be user?

It'll be called by our libs. E.g. CreatableUpdatableImpl.

I planned to also put this configuration in ResourceManagerUtils, where user can set ReactorScheduler in InternalContext. E.g. setSyncTaskExecutor.

Surely we can use ResourceManagerUtils directly in TaskGroup, though I thought a separation of layers could be useful(TaskGroup vs CreatableUpdatableImpl).

XiaofeiCao · 2025-04-29T04:06:31Z

Created a new PR with commits squashed.
#45139

Seems revapi issue has gone for that one...

XiaofeiCao · 2025-05-06T05:18:09Z

close by #45139

XiaofeiCao · 2025-05-06T08:15:39Z

Seems the revapi issue in pullrequest CI has been fixed. Will still use this PR.

XiaofeiCao added 11 commits April 16, 2025 08:58

sync TaskGroup

93a9598

run first

8f42bfe

test

4d246c5

javadocs, pass ProxyTaskGroupTests

e421670

completablefutures

e093a59

fix error handling

6eaf0e8

add sync-stack to error handling tests

216332f

fix

2a3fe33

javadoc

2e5accd

fix compatibility

2a5481c

javadocs

af3e26c

XiaofeiCao changed the title ~~mgmt resources, sync implementation~~ mgmt resources, sync implementation for TaskGroup Apr 22, 2025

github-actions bot added the Mgmt This issue is related to a management-plane library. label Apr 22, 2025

XiaofeiCao added 4 commits April 23, 2025 08:44

support executing tasks on current thread

b701fbf

format

982c6c3

support invokeDependency for TaskGroup

48bd5da

fix test when cpu core=1

274d621

XiaofeiCao added 3 commits April 24, 2025 21:39

log

1445dc4

more logs

99bd818

log

b7ac4b7

XiaofeiCao changed the title ~~mgmt resources, sync implementation for TaskGroup~~ [WIP] mgmt resources, sync implementation for TaskGroup Apr 25, 2025

separate sync and async tests

a6639f4

XiaofeiCao changed the title ~~[WIP] mgmt resources, sync implementation for TaskGroup~~ mgmt resources, sync implementation for TaskGroup Apr 25, 2025

XiaofeiCao added 5 commits April 25, 2025 17:12

remove unnecessary logs

14351bf

synchronize

00eddd1

change back ErroredDependencyTaskException to checked Exception

3a98109

log throwable as warning

7cdf4d5

fix javadoc

3abdf53

XiaofeiCao added 4 commits April 27, 2025 14:26

fix lint

29ae258

format

1b19243

Merge remote-tracking branch 'upstream/main' into mgmt_sync_task_group

7e6f3fc

make ErroredDependencyTaskException RuntimeException

e66894b

XiaofeiCao commented Apr 27, 2025

View reviewed changes

XiaofeiCao added 2 commits April 27, 2025 15:57

remove unnecessary APIs and fix sync tests

19a80cc

nit, import

04f3f45

XiaofeiCao marked this pull request as ready for review April 27, 2025 08:55

XiaofeiCao requested review from weidongxu-microsoft, haolingdong-msft and ArthurMa1978 as code owners April 27, 2025 08:55

weidongxu-microsoft reviewed Apr 28, 2025

View reviewed changes

address comments

e3aa426

XiaofeiCao closed this Apr 29, 2025

XiaofeiCao reopened this Apr 29, 2025

XiaofeiCao mentioned this pull request Apr 29, 2025

[NEW]mgmt resources, sync implementation for TaskGroup #45139

Closed

6 tasks

Merge branch 'Azure:main' into mgmt_sync_task_group

baa4e1f

XiaofeiCao added the Do Not Merge label Apr 29, 2025

XiaofeiCao changed the title ~~mgmt resources, sync implementation for TaskGroup~~ [DO NOT REVIEW]mgmt resources, sync implementation for TaskGroup Apr 29, 2025

XiaofeiCao marked this pull request as draft April 29, 2025 10:53

XiaofeiCao closed this May 6, 2025

XiaofeiCao reopened this May 6, 2025

Merge branch 'Azure:main' into mgmt_sync_task_group

e7cc230

XiaofeiCao marked this pull request as ready for review May 6, 2025 08:15

XiaofeiCao changed the title ~~[DO NOT REVIEW]mgmt resources, sync implementation for TaskGroup~~ mgmt resources, sync implementation for TaskGroup May 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

mgmt resources, sync implementation for TaskGroup #45068

mgmt resources, sync implementation for TaskGroup #45068

Uh oh!

XiaofeiCao commented Apr 22, 2025 •

edited

Loading

Uh oh!

azure-sdk commented Apr 22, 2025 •

edited by github-actions bot

Loading

Uh oh!

azure-sdk commented Apr 24, 2025

Uh oh!

XiaofeiCao Apr 27, 2025

Uh oh!

weidongxu-microsoft Apr 28, 2025

Uh oh!

XiaofeiCao commented Apr 27, 2025 •

edited

Loading

Uh oh!

weidongxu-microsoft left a comment

Uh oh!

weidongxu-microsoft Apr 28, 2025

Uh oh!

Uh oh!

weidongxu-microsoft Apr 28, 2025

Uh oh!

XiaofeiCao Apr 28, 2025 •

edited

Loading

Uh oh!

XiaofeiCao commented Apr 29, 2025 •

edited

Loading

Uh oh!

XiaofeiCao commented May 6, 2025

Uh oh!

XiaofeiCao commented May 6, 2025

Uh oh!

Uh oh!

mgmt resources, sync implementation for TaskGroup #45068

Are you sure you want to change the base?

mgmt resources, sync implementation for TaskGroup #45068

Uh oh!

Conversation

XiaofeiCao commented Apr 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Goal

Context

About this PR

Test

Future works

All SDK Contribution checklist:

General Guidelines and Best Practices

Testing Guidelines

Uh oh!

azure-sdk commented Apr 22, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

API Change Check

Uh oh!

azure-sdk commented Apr 24, 2025

Uh oh!

XiaofeiCao Apr 27, 2025

Choose a reason for hiding this comment

Uh oh!

weidongxu-microsoft Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

XiaofeiCao commented Apr 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

weidongxu-microsoft left a comment

Choose a reason for hiding this comment

Uh oh!

weidongxu-microsoft Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

weidongxu-microsoft Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

XiaofeiCao Apr 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

XiaofeiCao commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

XiaofeiCao commented May 6, 2025

Uh oh!

XiaofeiCao commented May 6, 2025

Uh oh!

Uh oh!

XiaofeiCao commented Apr 22, 2025 •

edited

Loading

azure-sdk commented Apr 22, 2025 •

edited by github-actions bot

Loading

XiaofeiCao commented Apr 27, 2025 •

edited

Loading

XiaofeiCao Apr 28, 2025 •

edited

Loading

XiaofeiCao commented Apr 29, 2025 •

edited

Loading