Skip to content
This repository has been archived by the owner on Jun 19, 2022. It is now read-only.

Have broker fanout handle timeout gracefully for each individual target #960

Merged
merged 4 commits into from
Apr 30, 2020

Conversation

yolocs
Copy link
Member

@yolocs yolocs commented Apr 28, 2020

Fixes #954

Proposed Changes

  • Fanout delivery for each target is now timeboxed. On timeout, it will send the event to retry queue. It has a slightly shorter timeout than the handler timeout. This is necessary because otherwise the handler timeout will cause the same event being re-delivered to all targets.

Release Note

Have broker fanout handle timeout gracefully for each individual target

Docs

@yolocs yolocs requested review from grac3gao-zz and liu-cong and removed request for ian-mi and capri-xiyue April 28, 2020 20:10
@googlebot googlebot added the cla: yes (override cla status due to multiple authors bug) label Apr 28, 2020
@yolocs
Copy link
Member Author

yolocs commented Apr 28, 2020

/retest

pkg/broker/handler/pool/fanout/pool_test.go Outdated Show resolved Hide resolved
pkg/broker/handler/processors/deliver/processor.go Outdated Show resolved Hide resolved
cmd/broker/fanout/main.go Show resolved Hide resolved
cmd/broker/fanout/main.go Show resolved Hide resolved
pkg/broker/handler/pool/fanout/pool.go Show resolved Hide resolved
pkg/broker/handler/pool/testing/helper.go Outdated Show resolved Hide resolved
pkg/broker/handler/pool/testing/helper.go Show resolved Hide resolved
@chizhg
Copy link
Member

chizhg commented Apr 28, 2020

/retest

@knative-metrics-robot
Copy link

The following is the coverage report on the affected files.
Say /test pull-google-knative-gcp-go-coverage to re-run this coverage report

File Old Coverage New Coverage Delta
pkg/broker/handler/pool/fanout/pool.go 79.6% 78.3% -1.3
pkg/broker/handler/pool/options.go 82.6% 85.2% 2.6
pkg/broker/handler/processors/deliver/processor.go 71.9% 75.7% 3.8

@chizhg
Copy link
Member

chizhg commented Apr 28, 2020

/retest

@chizhg
Copy link
Member

chizhg commented Apr 28, 2020

/test pull-google-knative-gcp-build-tests

@yolocs
Copy link
Member Author

yolocs commented Apr 29, 2020

/retest

@yolocs yolocs requested a review from liu-cong April 29, 2020 20:08
@grac3gao-zz
Copy link
Contributor

grac3gao-zz commented Apr 30, 2020

/lgtm
/hold
In case other reviewer may want to take another round of review.
Please feel free to un-hold it.

@liu-cong
Copy link
Contributor

/lgtm
/approve
/unhold

@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: liu-cong, yolocs

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot merged commit aa7d86b into google:master Apr 30, 2020
@yolocs yolocs deleted the b/delivertimeout branch April 30, 2020 17:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved cla: yes (override cla status due to multiple authors bug) lgtm size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Broker fanout: on a target delivery timeout, it should send event to retry queue
7 participants