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

Make ProcessorTimeout in Shutdown API Configurable #1742

Closed
engechas opened this issue Sep 8, 2022 · 2 comments · Fixed by #1757
Closed

Make ProcessorTimeout in Shutdown API Configurable #1742

engechas opened this issue Sep 8, 2022 · 2 comments · Fixed by #1757
Labels
enhancement New feature or request
Milestone

Comments

@engechas
Copy link
Collaborator

engechas commented Sep 8, 2022

Is your feature request related to a problem? Please describe.
The time for processors to flush their data downstream and shutdown is hardcoded to 10 seconds. This may or may not be enough time for a processor to clear its data and prevent data loss.

Describe the solution you'd like
Make the shutdown timeout configurable via the data-prepper-config.yaml

Describe alternatives you've considered (Optional)
Adding the timeout as a parameter to the shutdown API.

@dlvenable
Copy link
Member

This is a good idea. I have a couple questions and comments.

  • What should this property be named?
  • This does give processors time to flush, but also sinks it seems. Should the name reflect more than just the processor timeout?

shutdownExecutorService(sinkExecutorService, processorTimeout);

  • I'd like to see this use the Data Prepper duration format. That is, it can be either an ISO-8601 string, a time in seconds (ending in s), or a time in milliseconds (ending in ms). Our plugins can support this, but the Data Prepper configuration file does not yet. I have a similar ask for new Core Peer Forwarding timeouts.

@engechas
Copy link
Collaborator Author

What should this property be named?
This does give processors time to flush, but also sinks it seems. Should the name reflect more than just the processor timeout?

I think it makes sense to have a separate property for sink and processor shutdown timeouts. My thoughts on property naming would be processorShutdownTimeout and sinkShutdownTimeout.

I'd like to see this use the Data Prepper duration format

This makes sense from a consistency standpoint. I'll look into the effort required to support this type in the data prepper config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants