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

Aws::SQS::QueuePoller automatically call change_message_visibility_timeout while work is in progress #2477

Open
michaelwittig opened this issue Feb 11, 2021 · 7 comments
Labels
feature-request A feature should be added or improved.

Comments

@michaelwittig
Copy link

Is your feature request related to a problem? Please describe.
I use the Aws::SQS::QueuePoller to get messages from SQS. I can either define an :visibility_timeout or the queue's default value is used. I can also call poller.change_message_visibility_timeout(msg, 60) to add another timeout if I need more time. But in situations where the processing is an expensive blocking call, I can not easily do that (e.g., download a large file from S3).

Describe the solution you'd like
So I was asking myself, why can't the poller automatically call change_message_visibility_timeout every x seconds to reset the timeout x+y while the work is in progress (code has neither returned or thrown).

Describe alternatives you've considered
One idea I had is to start a Thread inside the block that calls the change_message_visibility_timeout from time to time until I kill it when I reached the end of my code block.

[ ] 👋 I may be able to implement this feature request

Additional context
Add any other context or screenshots about the feature request here.

@michaelwittig michaelwittig added the feature-request A feature should be added or improved. label Feb 11, 2021
@mullermp mullermp added the needs-triage This issue or PR still needs to be triaged. label Feb 11, 2021
@github-actions
Copy link

Greetings! We’re closing this issue because it has been open a long time and hasn’t been updated in a while and may not be getting the attention it deserves. We encourage you to check if this is still an issue in the latest release and if you find that this is still a problem, please feel free to comment or open a new issue.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Feb 12, 2022
@mullermp mullermp removed the closing-soon This issue will automatically close in 4 days unless further comments are made. label Feb 14, 2022
@mullermp mullermp removed the needs-triage This issue or PR still needs to be triaged. label Mar 11, 2022
@JohnLegrandRichards
Copy link

JohnLegrandRichards commented Oct 28, 2022

I would be willing to look into this @mullermp. Are there any issues with adding functionality to these generated files?

@mullermp
Copy link
Contributor

Thanks for reaching out about this. QueuePoller is not code generated, so you are free to change the file. As far as the feature request goes, on the surface, having a managed thread that updates the timeout until the block has returned might be ok, but it definitely needs to be an opt-in feature. I don't like the possibility that the poller could be blocked infinitely with no timeout - we still should configure a maximum. AFAIK the maximum visibility timeout is already 12 hours - are there really s3 downloads that take that long? I am doubting the need for this feature request. @JohnLegrandRichards are you by chance running into an issue in your application that requires this feature?

@mullermp
Copy link
Contributor

I am closing this because 12 hours is an extremely generous maximum visibility timeout. We will re-consider with more customer data.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@michaelwittig
Copy link
Author

@mullermp the point is not that something takes longer than 12 hours.

The point is that you can consistently tell SQS that the message is still worked on. In case a worker dies, the messages is quickly picked up by another worker. Instead of waiting for the 12 hours to end (or whatever the visibility timeout was).

@mullermp
Copy link
Contributor

That makes sense, sorry, I will re-open this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

3 participants