openshift / geard

geard is no longer maintained - see OpenShift 3 and Kubernetes
Other
405 stars 80 forks source link

Fix issue #187 where geard freeze after subsequent stop/start #193

Closed mfojtik closed 10 years ago

mfojtik commented 10 years ago

@smarterclayton fyi

I tested this with a loop that perform stop/start 100x and it seems to work fine.

One thing I don't understand is why the out <- result work when the result is "done" and not when the result is "canceled". It seems like in StartJob we are dropping the channel when we call the function and I handle the case in StopJob where we actually care about the retval.

smarterclayton commented 10 years ago

We should close the channel maybe, or attack a function that will read it silently then exit.

mfojtik commented 10 years ago

@smarterclayton the channel is already closed (I got 'closing an already closed channel panic). So I think we just need to check if the result is 'canceled' and then skip sending the result to this channel.

@smarterclayton also I tried to follow the c.jobListener.jobs[job] to what closes the channel, but no luck :-( It must be somewhere in the godbus or in the alter_container_state). But I think the current patch is OK.

smarterclayton commented 10 years ago

Can you create a test case that reproduces this? A benchmark or otherwise that we run during integration tests.

mfojtik commented 10 years ago

@smarterclayton sure

mfojtik commented 10 years ago

@smarterclayton ignore the test right now ;-) just checking it in to make it run in Jenkins.

mfojtik commented 10 years ago

[test]

openshift-bot commented 10 years ago

Origin Test Results: FAILURE (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_geard/113/)

smarterclayton commented 10 years ago

[merge]

openshift-bot commented 10 years ago

Origin Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_requests_geard/22/) (Image: devenv-fedora_81)

openshift-bot commented 10 years ago

Evaluated for origin up to 095edf096081ceb84b2809873f58e4af96a9cf10