-
Notifications
You must be signed in to change notification settings - Fork 188
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
Variable drain timeouts when shutting down over HTTP shutdown. #4970
base: main
Are you sure you want to change the base?
Variable drain timeouts when shutting down over HTTP shutdown. #4970
Conversation
Adds two new parameters to the shutdown API. The first is bufferReadTimeout which controls the amount of time to wait for the buffer to be empty. The second is bufferDrainTimeout which controls the overall wait time for the process worker threads to complete. To support Data Prepper durations in HTTP query parameters, I extracted the parsing logic for durations out of DataPrepperDurationDeserializer and into a new DataPrepperDurationParser class. Resolves opensearch-project#4966. Signed-off-by: David Venable <[email protected]>
Signed-off-by: David Venable <[email protected]>
|
||
final Duration bufferReadTimeout = dataPrepperShutdownOptions.getBufferReadTimeout(); | ||
if(bufferReadTimeout != null) { | ||
forceStopReadingBuffersAt = shutdownRequestedAt.plus(bufferReadTimeout); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this only apply to the Kafka buffer? If this times out reading from in-memory buffer there is a chance of data loss right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would apply to any buffer. But, it has to be provided in the /shutdown
API request. So a user could force a faster shutdown even with the in-memory buffer using this.
bufferDrainTimeoutOverride : bufferDrainTimeout; | ||
} | ||
|
||
private Instant now() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This is returning the pipeline shutdown request time and not the current time ? We should consider renaming this.
Description
Adds two new parameters to the shutdown API. The first is bufferReadTimeout which controls the amount of time to wait for the buffer to be empty. The second is bufferDrainTimeout which controls the overall wait time for the process worker threads to complete.
To support Data Prepper durations in HTTP query parameters, I extracted the parsing logic for durations out of DataPrepperDurationDeserializer and into a new DataPrepperDurationParser class.
Issues Resolved
Resolves #4966.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.