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

Removes user executor #16215

Merged
merged 4 commits into from
Dec 17, 2019

Conversation

vbekiaris
Copy link
Contributor

@vbekiaris vbekiaris commented Dec 10, 2019

Removes user executor and associated config, as discussed here

@vbekiaris vbekiaris force-pushed the fixes/4.0/remove-user-executor branch from 2f55adb to b9e71f9 Compare December 12, 2019 16:45
@vbekiaris vbekiaris changed the title [WIP] remove user executor Removes user executor Dec 12, 2019
@vbekiaris vbekiaris self-assigned this Dec 12, 2019
@vbekiaris vbekiaris added this to the 4.0 milestone Dec 12, 2019
@vbekiaris vbekiaris marked this pull request as ready for review December 12, 2019 16:48
@vbekiaris vbekiaris requested a review from a team as a code owner December 12, 2019 16:48
@vbekiaris
Copy link
Contributor Author

run-lab-run

1 similar comment
@vbekiaris
Copy link
Contributor Author

run-lab-run

@sancar sancar self-requested a review December 16, 2019 10:12
@@ -170,7 +171,7 @@ public ClientTransactionManagerService getTransactionManager() {
return transactionManager;
}

public ClientExecutionService getExecutionService() {
public TaskScheduler getExecutionService() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can remove ClientExecutionService interface completely and also change the method name to getTaskScheduler . There seems to be no use left for ClientExecutionService interface anymore.

@vbekiaris vbekiaris force-pushed the fixes/4.0/remove-user-executor branch from b9e71f9 to f1bfe39 Compare December 16, 2019 16:09
@kwart kwart self-requested a review December 17, 2019 08:12
User callbacks are now executed by default in the DEFAULT_ASYNC executor.
The few remaining usages of user executor have been replaced and user
executor along with its configuration is removed.
@vbekiaris vbekiaris force-pushed the fixes/4.0/remove-user-executor branch from 5682e09 to 4fa5d6c Compare December 17, 2019 08:18
Copy link
Member

@kwart kwart left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only minor comments. Otherwise LGTM.

@@ -313,7 +313,7 @@ public String toString() {
}
}));
executor.shutdownIncoming();

System.out.println(">>>> >>>>>");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover?

@@ -329,6 +329,7 @@ public void run() {
}
});
executor.shutdownOutgoing();
System.out.println("<<<< <<<<");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Leftover?

@vbekiaris
Copy link
Contributor Author

vbekiaris commented Dec 17, 2019

PR builder failed with known issue #16099

@vbekiaris vbekiaris merged commit 73774bc into hazelcast:master Dec 17, 2019
@vbekiaris vbekiaris deleted the fixes/4.0/remove-user-executor branch December 17, 2019 11:09
@mmedenjak mmedenjak added the Source: Internal PR or issue was opened by an employee label Apr 13, 2020
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants