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.

Sign up for GitHub

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

Jump to bottom

Creating a Disruptor instance with an Executor is deprecated in favour of a ThreadFactory #567

Closed
brenuart opened this issue Jul 9, 2021 · 0 comments · Fixed by #570
Closed

Creating a Disruptor instance with an Executor is deprecated in favour of a ThreadFactory #567

brenuart opened this issue Jul 9, 2021 · 0 comments · Fixed by #570
Labels
type/enhancement
Milestone
7.0

Comments

@brenuart
Copy link
Collaborator

brenuart commented Jul 9, 2021 •
edited
Loading

The AsyncDisruptorAppender creates Disruptor instances using the constructor that takes an Executor as argument.
This constructor has been deprecated in 2015 in favour of a ThreadFactory. The deprecation comments says:

Use a {@link ThreadFactory} instead of an {@link Executor} as a ThreadFactory is able to report errors when it is unable to construct a thread to run a producer.

The deprecation has been introduced by commit LMAX-Exchange/disruptor@4273056 and is in effect since version 3.3.4.

The AsyncDisruptorAppender already maintains a ThreadFactory. I don't see any problem in passing it to the Disruptor instead of the executor service.
brenuart added a commit to brenuart/logstash-logback-encoder that referenced this issue Jul 14, 2021
@brenuart
Create Disruptor with a ThreadFactory instead of ExecutorService
85128ac 
See logfellow#567
@brenuart brenuart mentioned this issue Jul 14, 2021
Merged
brenuart added a commit to brenuart/logstash-logback-encoder that referenced this issue Jul 20, 2021
@brenuart
Create Disruptor with a ThreadFactory instead of ExecutorService
e7df7c3 
See logfellow#567
philsttr pushed a commit that referenced this issue Jul 25, 2021
@brenuart
Create Disruptor with a ThreadFactory instead of ExecutorService (#570)
6386d2a 
* Create Disruptor with a ThreadFactory instead of ExecutorService
* Remove ExecutorService from AsyncDisruptorAppender and move it inside the AbstractLogstashTcpSocketAppender
  AsyncDisruptorAppender does use the ExecutorService anymore - move it inside classes that need it

See #567
@philsttr philsttr linked a pull request Jul 25, 2021 that will close this issue
Create Disruptor with a ThreadFactory instead of ExecutorService #570
Merged
@philsttr philsttr added this to the 7.0 milestone Jul 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/enhancement
Projects
None yet
Milestone
7.0
Development

Successfully merging a pull request may close this issue.

Create Disruptor with a ThreadFactory instead of ExecutorService brenuart/logstash-logback-encoder
2 participants
@brenuart @philsttr