Enforce 12 hour maxJobDuration #275
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jakepruitt
approved these changes
Aug 8, 2018
| @@ -63,9 +63,10 @@ const dashboard = require(path.resolve(__dirname, 'dashboard.js')); | |||
| * number or a reference, i.e. `{"Ref": "..."}`. | |||
| * @param {Boolean} [options.privileged=false] - give the container elevated | |||
| * privileges on the host container instance | |||
| * @param {Number|ref} [options.maxJobDuration] - the maximum number of seconds | |||
| * @param {Number|ref} [options.maxJobDuration] - the number of seconds | |||
There was a problem hiding this comment.
Should we say that the default is 12 hours?
| @@ -103,6 +103,14 @@ In writableFilesystem mode, the whole file system is writable and containers are | |||
|
|
|||
| writableFilesystem mode has no restrictions to the file system: workers can write anywhere and read from anywhere, their files being instantly deleted after the job finishes and the container dies. | |||
|
|
|||
| ### maxJobDuration explained | |||
|
|
|||
| When maxJobDuration is exceeded by a worker, the worker will stop all processes and return the processing message to the queue. Typically, the maxJobDuration should be set marginally above the maximum time that it takes your application to process and delete a message from the queue. This ensures all work can be completed; while providing protection against erroneous processing continuing to the maximum of twelve hours. | |||
There was a problem hiding this comment.
I'm likely confused but it looks like if maxJobDuration is exceeded, the behavior is the same as when SQS hard limit of 12 hours is exceeded, because in both cases the message is returned to the queue, is that right?
Or will a worker keep working when the 12-hour visibility timeout is breached?
There was a problem hiding this comment.
Or will a worker keep working when the 12-hour visibility timeout is breached?
Yes, I believe the worker will keep working regardless of hitting the timeout. We could implement a hard cutoff, but that may or may not be desirable depending on the application context.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Opening a pr to close the loop on timeout handling.
As it stands right now watchbot will duplicate work if the message takes longer than 12 hours to process and no timeout is set. Quick solution is to acknowledge the current 12 hour limitation by enforcing maxJobDuration on workers.
I've chosen 43020 as the max (12 hours minus three minutes) to avoid edge cases that could occur if the heartbeat tries to extend the visibility timeout 3 minutes and the total would be over 43200 seconds. Also attempted some docs to explain this limitation.
cc: @jakepruitt @rclark
closes #203