feat: implement acknowledgment tracking for pipeline records#79
feat: implement acknowledgment tracking for pipeline records#79Mahesh Kamble (ma-gk) wants to merge 1 commit into
Conversation
| // readers are forward-only, so counting takes its own pass over a | ||
| // fresh reader. | ||
| regularFiles := 0 | ||
| for counter := tar.NewReader(bytes.NewReader(b)); ; { |
There was a problem hiding this comment.
loop on L45 and L61 is very similar, can we merge that to a single loop?
|
Nice approach! Then we can simply call these funcs. |
| // wait for: delete the receipt right away, same as when | ||
| // there's no consumer at all. | ||
| if output == nil { | ||
| if _, err := s.client.DeleteMessage(ctx, &qs.DeleteMessageInput{ |
There was a problem hiding this comment.
Are we deleting message as soon as it is received, and then again calling s.deleteOnComplete?
| s.SendData(ack.WithContext(ctx, msgAck), []byte(*m.Body), output) | ||
|
|
||
| wg.Add(1) | ||
| go s.deleteOnComplete(msgAck, m.MessageId, m.ReceiptHandle, inFlight, wg) |
There was a problem hiding this comment.
Calling a go routine for every message can cause unbounded growth in go routines, can we fall back to previous pattern where we are having go routines based on concurrency?
| s.SendData(ack.WithContext(ctx, msgAck), []byte(*m.Body), output) | ||
|
|
||
| wg.Add(1) | ||
| go s.deleteOnComplete(msgAck, m.MessageId, m.ReceiptHandle, inFlight, wg) |
There was a problem hiding this comment.
For larger record counts, it is going to create unbounded count of go routines as each is going to create a channel in blocking state, instead we can use a optimised approach on callback func, and the sink task then executes it.
Same can be done for kafka as well.
// In getMessages — no goroutines, no waiting, no channels:
for _, m := range receiveMessageOutput.Messages {
msgAck := ack.New()
// capture receipt handle; delete runs inline on whichever
// goroutine calls the final Done/Fail — zero extra goroutines.
receipt := m.ReceiptHandle
msgId := m.MessageId
msgAck.Done(func(failed bool) {
if !failed {
s.client.DeleteMessage(ctx, &qs.DeleteMessageInput{
QueueUrl: &s.QueueURL,
ReceiptHandle: receipt,
})
}
})
s.SendData(ack.WithContext(ctx, msgAck), []byte(*m.Body), output)
}
And then, in the end on termination you can call
msgAck.Done(true)
With this approach, now deletion would not be enqueued.
There was a problem hiding this comment.
Something same can be done for kafka.
We can define an OnComplete function in interface, which can do cleanup
Description
Implements acknowledgment tracking for pipeline records so an SQS source only deletes a message's receipt once every downstream branch derived from it has finished processing — instead of deleting it as soon as it's received.
internal/pkg/pipeline/ackpackage: anAcktracks how many pending "branches" a source record has, supports fan-out (AddBranch/Fanout), fan-in (Joined), drop (Drop), and per-branch success/failure (Done/Fail), and rides downstream attached torecord.Record.Context.pipeline.go'sdistributeToChannelsnow callsack.Fanoutfor structural DAG fan-out (same record duplicated across parallel branches).sqssource task: replaced the old fixed worker-pool receipt-deletion model with per-messagedeleteOnComplete, which waits on the message'sAckand only deletes the receipt if every downstream branch succeeded; a failed branch leaves the receipt alone so SQS redelivers it after the visibility timeout.test/pipelines/sqs_ack_*_test.yaml,run_localstack_ack_scenarios.sh,setup_localstack_sqs.sh) covering baseline, fan-out (split/DAG/jq-explode/archive-unpack), fan-in (join/archive-pack), and drop (jq/sample) scenarios.Types of changes
Checklist