Feature: SQS Heartbeating

[kylez-ithaka]

📢 Type of change

• Bugfix
• New feature
• Enhancement
• Refactoring

📜 Description

Implements / Fixes #1554 : Implement heartbeat for long running tasks.

This allows a consumer of SQS which doesn't know up front the maximum time it will process a message, to use a "heartbeat" mechanism where it periodically checks back in with SQS to extend the visibility timeout, per AWS best practices https://docs.aws.amazon.com/AWSSimpleQueueService/latest/SQSDeveloperGuide/sqs-visibility-timeout.html

This has benefits over setting a really long maximum visibility timeout

• If process fails long before maximum visibility timeout you set, it reduces latency of the message getting picked back up by a consumer, or moving into the dead letter queue (if configured)
• If process goes beyond the maximum visibility timeout, but is still in progress, it prevents the message from getting picked up by another consumer, which would've caused extra work/cost and potentially other issues (including dead lettering things that actually completed)
• Reduces effort to implement SQS listeners overall, you can set the default visibility timeout for rough expected amount for it to complete processing (or just default of 30 seconds, or your own standard default), as long as you setup the heartbeat to happen before visibility timeout

The costs over setting a really long maximum visibility timeout

• There is overhead of making the heartbeat, it's a rather small overhead individually (simple call to SQS, single future), but if configured too frequently and running large amounts of SQS messages on the same server, it could add up (especially if you under provision for your CPU and misconfigure the heartbeat to happen too frequently)

💡 Motivation and Context

This is pretty standard practice for SQS, especially for longer or variable time processing. Motivating by having issues with messages ending up in dead letter because timeout wasn't long enough for some items, and caused it to be processed multiple times

💚 How did you test it?

Unit tests, and I made a queue and ran it locally, I setup a queue and put messages on it, that normally would timeout but now doesn't. I could see the logs about heart beating as well.

Note this was to test it out, one shouldn't do a heartbeat every 1 second

 @SqsListener(value = "test-sqs-heartbeat-queue", messageVisibilityHeartbeatIntervalSeconds ="1",messageVisibilityHeartbeatSeconds = "30")
    public void listenToTest(String message) {
        LOGGER.info("Start processing {}", message);
        for(int i = 0; i < 10; i++){
            try {
                LOGGER.info("Processing message, task #{} - {}", i, message);
                Thread.sleep(6000);
            } catch (InterruptedException e) {}
        }
        LOGGER.info("End processing {}", message);
    }

📝 Checklist

• I reviewed submitted code
• I added tests to verify changes
• I updated reference documentation to reflect the change
• All tests passing
• No breaking changes

🔮 Next steps

Make some release that has it in it, and include in changelog?

[kylez-ithaka]

this would be a type:feature or type:enhancement, not documentation

[kylez-ithaka]

@maciejwalkowiak @MatejNedic @tomazfernandes is there anything I should adjust for this to get it queued up for a Review?

[tomazfernandes]

Hi @kylez-ithaka, thanks for the PR.

I've started reviewing it already, will get back to you soon.

[tomazfernandes]

@kylez-ithaka thanks for the PR.

The main issue I see is that scheduling the heartbeat at intercept level only covers messages that have started processing. For ordered sinks (FIFO queues), messages from the same batch wait for their turn, so their visibility won't be extended when a previous message goes long.

We could instead schedule a single heartbeat for the whole batch in the Sink when the messages are emitted, and use a lightweight interceptor only to signal that the message is done processing and remove it from the heartbeat set.

What do you think?

[kylez-ithaka]

@tomazfernandes good catch and that makes sense to be the change, I don't know if i'll get around to that update until a few weeks from now