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

source.Channel ignoring event #942

Closed
lrgar opened this issue May 11, 2020 · 4 comments · Fixed by #1146
Closed

source.Channel ignoring event #942

lrgar opened this issue May 11, 2020 · 4 comments · Fixed by #1146
Assignees
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@lrgar
Copy link

lrgar commented May 11, 2020

Hello, I'm using source.Channel to do an artificial initial reconciliation followed by periodic cycles, and have noticed that sometimes the initial request is ignored.

Did some investigation and looks like a race condition where the events are processed before the Channel has any targets.

In particular this goroutine is started,

cs.once.Do(func() {
// Distribute GenericEvents to all EventHandler / Queue pairs Watching this source
go cs.syncLoop()
})

before,

cs.dest = append(cs.dest, dst)

Here, syncLoop() calls distribute() who sends the event to every channel on cs.dest. So it can happen that that syncLoop() processes an event, before cs.dest is initialized, so it doesn't get processed by anyone.

As for a fix, I'd suggest starting the goroutine after cs.dest is set instead. I can create a PR as well if preferred.

This was seen on controller-runtime v0.5.2, but looking at the code, it seems to also apply to master.

Best regards,
Luis

@vincepri
Copy link
Member

/kind bug
/milestone v0.7.x
/priority important-soon
/help

@k8s-ci-robot
Copy link
Contributor

@vincepri:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

/kind bug
/milestone v0.7.x
/priority important-soon
/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jul 22, 2020
@k8s-ci-robot k8s-ci-robot added this to the v0.7.x milestone Jul 22, 2020
@k8s-ci-robot k8s-ci-robot added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. labels Jul 22, 2020
@lrgar
Copy link
Author

lrgar commented Jul 27, 2020

I can take this one then.

@alvaroaleman
Copy link
Member

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants