google / go-cloud

The Go Cloud Development Kit (Go CDK): A library and tools for open cloud development in Go.
https://gocloud.dev/
Apache License 2.0
9.57k stars 811 forks source link

pubsub: add support for extending ACK deadline #3507

Open h27771420 opened 1 week ago

h27771420 commented 1 week ago

Is your feature request related to a problem? Please describe.

I guess I have to start with the differences between AWS SQS and GCP Pub/Sub, here are what I think are the more significant differences between them

  1. SQS accepts Ack queue message after the visibility timeout is exceeded.
  2. But it does not work for Pub/Sub. If the Ack Deadline is exceeded, our Ack operation will fail.
  3. The upper limit of visibility timeout of SQS is 12 hours.
  4. The upper limit of Ack deadline of Pub/Sub is 10 minutes.

So here is my current situation, I'm migrating our library from the AWS SDK to go-cloud to invoke Pub/Sub. (Eventually I would like to make it works like a multi-cloud/multi-queue-service support.) (Btw, I really have to say, go-cloud is truly amazing. 😄 ) But, the design logic in all my previous services was basically Queue-message-driven. I usually poll the message first and then start processing the message. When the processing is completed, I would decide whether to Ack or Nack.

I know that though they are essentially different services, the above differences are too huge, so when processing some time-consuming requests, 10 minutes is often not enough. (Pub/Sub also not allow us to Ack after reached the deadline. 😔 ) In the end my service usually can handle that message successfully, but because that message cannot be correctly Acked, that message will be judged as a processing failure by Pub/Sub, and will eventually be automatically transferred to the dead letter topic.

Describe the solution you'd like

So I hope go-cloud can add a new method to extend Ack deadline.

func (m *pubsub.Message) ExtendAckDeadline(d time.Duration) error {...}

Describe alternatives you've considered

The alternative I guess would be For polling:

  1. As soon as I poll the message, I'd need to cache it into Redis or somewhere, and like marking it is an on flying message. (Ah, a point just came into my mind. In that case, why don't I just stop using Pub/Sub in the first place? Eventually I still have to rewrite the entire message queue mechanism.)

For handling:

  1. Pop out the message from that queue cache(or storage), handle it.
  2. if succeeded, do nothing.
  3. if failed, push it back into queue, but add 1 failure count.
  4. if reached the max retry times, deliver it to the corresponding dead letter topic.

(However, there is still a problem. If it panic, the whole thing will be ruined, so another thing may be needed to provide to prevent that request from disappearing forever after panicing.)

Additional context

Sorry, I seem to have written context in the wrong place, please refer to above.

vangent commented 6 days ago

Ack; we've considered this before (https://github.com/google/go-cloud/issues/3507).

h27771420 commented 4 days ago

@vangent Ahhhhh, thanks for your response, I guess you mean https://github.com/google/go-cloud/issues/3130? And that AckID should be abled to invoke ModifyAckDeadline.

I saw the reason, but in my case, it feels weird to open another google/pubsub client to make this request. It is like opening the Pub/Sub client twice at the same time, I really don't like to do that. 😔

Or could it be an option in pubsub/gcppubsub to decide whether to automatically extend Ack deadline, or let the Subscription client extend a specific time for Ack deadline? (e.g., A MaxExtension option in google/pubsub https://github.com/googleapis/google-cloud-go/issues/608)

For go-cloud, maybe

  url := "gcppubsub://...?MaxExtension=3000" // sec
vangent commented 4 days ago

You can certainly do this with As.

3507 was more about modifying the gcppubsub provider to support automatically extending ack deadlines, similar to what the higher-level GCP pubsub client does (there are references in #3507). It doesn't look like this functionality makes sense for pubsub as a whole, but it might be reasonable for the GCP pubsub provider specifically. I will consider it; in the meantime if you're stuck you can do this yourself as described above.

h27771420 commented 4 days ago

Awesome, thanks for pointing out the pubsub.Subscription.As(), I think this could be quite elegant.