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

Some annotated service methods are run from blocking task executors. #2078

Closed
trustin opened this issue Sep 16, 2019 · 1 comment · Fixed by #2187
Closed

Some annotated service methods are run from blocking task executors. #2078

trustin opened this issue Sep 16, 2019 · 1 comment · Fixed by #2187

Comments

@trustin
Copy link
Member

trustin commented Sep 16, 2019

Related: #2064

For example, when the return type of a service method is AggregatedHttpResponse or response object conversion is involved, the service method is currently executed via the blocking task executor. By contrast, if a service method returns HttpResponse, it will be executed from a event loop thread. This behavioral difference is too subtle and confusing.

We need to:

  • Make the service methods run from an event loop thread by default, regardless of their return types.
  • Introduce an annotation that makes the service methods run from a blocking task executor.
  • Provide some migration plan, like:
    • Introduce a flag that enables this change, whose default is false (disabled).
    • When the flag is disabled, issue a warning log message to tell people to migrate.
    • After a few releases, change the default from false to true.
@heowc
Copy link
Contributor

heowc commented Oct 13, 2019

I will work on it!

minwoox pushed a commit that referenced this issue Oct 24, 2019
Motivation:
The annotated services whose return type is neither `HttpResponse` nor `CompeltableFuture`, are run using a `blockingTaskExecutor` by default.
We should fix this to use `EventLoop` by default and let a user choose to use `blockingTaskExecutor` if he/she wants.

Modifications:
- Add `@Blocking` which makes the annotated service run using `blockingTaskExecutor`
- Make all annotated services run from `EventLoop`

Result:
- Fix #2078
- (Breaking) All annotated services are now run from `EventLoop` by default
eugene70 pushed a commit to eugene70/armeria that referenced this issue Nov 10, 2019
…e#2187)

Motivation:
The annotated services whose return type is neither `HttpResponse` nor `CompeltableFuture`, are run using a `blockingTaskExecutor` by default.
We should fix this to use `EventLoop` by default and let a user choose to use `blockingTaskExecutor` if he/she wants.

Modifications:
- Add `@Blocking` which makes the annotated service run using `blockingTaskExecutor`
- Make all annotated services run from `EventLoop`

Result:
- Fix line#2078
- (Breaking) All annotated services are now run from `EventLoop` by default
fmguerreiro pushed a commit to fmguerreiro/armeria that referenced this issue Sep 19, 2020
…e#2187)

Motivation:
The annotated services whose return type is neither `HttpResponse` nor `CompeltableFuture`, are run using a `blockingTaskExecutor` by default.
We should fix this to use `EventLoop` by default and let a user choose to use `blockingTaskExecutor` if he/she wants.

Modifications:
- Add `@Blocking` which makes the annotated service run using `blockingTaskExecutor`
- Make all annotated services run from `EventLoop`

Result:
- Fix line#2078
- (Breaking) All annotated services are now run from `EventLoop` by default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants