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

fix logic to wait for executor stop #392

Merged
merged 2 commits into from
Oct 27, 2022

Conversation

hiroebe
Copy link

@hiroebe hiroebe commented Oct 26, 2022

What does this do?

Add stoppedCh aside from stopCh, which is used to wait for the executor to stop

Which issue(s) does this PR fix/relate to?

Resolves #391.

List any changes that modify/break current functionality

Have you included tests for your changes?

No

Did you document any new/modified functionality?

  • Updated example_test.go
  • Updated README.md

Notes

limitMode limitMode
maxRunningJobs *semaphore.Weighted
}

func newExecutor() executor {
return executor{
jobFunctions: make(chan jobFunction, 1),
stopCh: make(chan struct{}, 1),
stopCh: make(chan struct{}),
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need capacity for this channel. Closing the channel is enough to tell the stop.

Comment on lines -123 to +126
e.stopCh <- struct{}{}
<-e.stopCh
close(e.stopCh)
<-e.stoppedCh
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need two channels for this use case. In the previous implementation, struct{}{} sent at L123 might be received at L124, which means start() loop never stops.

@JohnRoesler
Copy link
Contributor

something the panic handler test is hosed. i'll try and look at it later - unless you beat me to it 😄

@hiroebe
Copy link
Author

hiroebe commented Oct 27, 2022

@JohnRoesler
I've added a commit to fix the test. 474913e
stop() must be called after the jobFunction has started. Otherwise, jobFunction is never called and the test never finishes.

@JohnRoesler JohnRoesler merged commit 1db6f41 into go-co-op:main Oct 27, 2022
@hiroebe hiroebe deleted the fix-stop-channel branch October 27, 2022 09:50
estahn pushed a commit to estahn/k8s-image-swapper that referenced this pull request Dec 1, 2022
## [1.3.3](v1.3.2...v1.3.3) (2022-12-01)

### ⬆️ Dependencies

* **deps:** Bump alpine from 3.16.2 to 3.16.3 ([#388](#388)) ([ffae497](ffae497))
* **deps:** Bump alpine from 3.16.3 to 3.17.0 ([#395](#395)) ([d32255d](d32255d))
* **deps:** Bump github.com/aws/aws-sdk-go from 1.44.126 to 1.44.136 ([#391](#391)) ([61a6ae2](61a6ae2)), closes [#4620](https://github.com/estahn/k8s-image-swapper/issues/4620) [#4619](https://github.com/estahn/k8s-image-swapper/issues/4619) [#4617](https://github.com/estahn/k8s-image-swapper/issues/4617) [#4616](https://github.com/estahn/k8s-image-swapper/issues/4616) [#4615](https://github.com/estahn/k8s-image-swapper/issues/4615) [#4614](https://github.com/estahn/k8s-image-swapper/issues/4614) [#4613](https://github.com/estahn/k8s-image-swapper/issues/4613) [#4611](https://github.com/estahn/k8s-image-swapper/issues/4611) [#4608](https://github.com/estahn/k8s-image-swapper/issues/4608) [#4609](https://github.com/estahn/k8s-image-swapper/issues/4609)
* **deps:** Bump github.com/aws/aws-sdk-go from 1.44.136 to 1.44.146 ([#397](#397)) ([d4a6136](d4a6136)), closes [#4638](https://github.com/estahn/k8s-image-swapper/issues/4638) [#4636](https://github.com/estahn/k8s-image-swapper/issues/4636) [#4633](https://github.com/estahn/k8s-image-swapper/issues/4633) [#4632](https://github.com/estahn/k8s-image-swapper/issues/4632) [#4630](https://github.com/estahn/k8s-image-swapper/issues/4630) [#4628](https://github.com/estahn/k8s-image-swapper/issues/4628) [#4627](https://github.com/estahn/k8s-image-swapper/issues/4627) [#4626](https://github.com/estahn/k8s-image-swapper/issues/4626) [#4625](https://github.com/estahn/k8s-image-swapper/issues/4625) [#4624](https://github.com/estahn/k8s-image-swapper/issues/4624)
* **deps:** Bump github.com/containers/image/v5 from 5.23.0 to 5.23.1 ([#393](#393)) ([84f4d18](84f4d18))
* **deps:** Bump github.com/go-co-op/gocron from 1.17.1 to 1.18.0 ([#390](#390)) ([1750ee9](1750ee9)), closes [go-co-op/gocron#388](go-co-op/gocron#388) [go-co-op/gocron#389](go-co-op/gocron#389) [go-co-op/gocron#392](go-co-op/gocron#392) [go-co-op/gocron#394](go-co-op/gocron#394) [go-co-op/gocron#393](go-co-op/gocron#393) [go-co-op/gocron#392](go-co-op/gocron#392) [go-co-op/gocron#394](go-co-op/gocron#394) [#393](#393) [#394](#394) [#392](#392) [#389](#389)
* **deps:** Bump github.com/gruntwork-io/terratest from 0.40.24 to 0.41.3 ([#398](#398)) ([ab35b1a](ab35b1a)), closes [gruntwork-io/terratest#1203](gruntwork-io/terratest#1203) [gruntwork-io/terratest#1202](gruntwork-io/terratest#1202) [gruntwork-io/terratest#1201](gruntwork-io/terratest#1201) [gruntwork-io/terratest#1199](gruntwork-io/terratest#1199) [gruntwork-io/terratest#1196](gruntwork-io/terratest#1196) [#1202](https://github.com/estahn/k8s-image-swapper/issues/1202) [#1203](https://github.com/estahn/k8s-image-swapper/issues/1203) [#1201](https://github.com/estahn/k8s-image-swapper/issues/1201) [#1199](https://github.com/estahn/k8s-image-swapper/issues/1199) [#1196](https://github.com/estahn/k8s-image-swapper/issues/1196)
* **deps:** Bump github.com/prometheus/client_golang from 1.13.0 to 1.13.1 ([#387](#387)) ([b155a16](b155a16)), closes [#1146](https://github.com/estahn/k8s-image-swapper/issues/1146) [#1148](https://github.com/estahn/k8s-image-swapper/issues/1148) [#1118](https://github.com/estahn/k8s-image-swapper/issues/1118) [#1146](https://github.com/estahn/k8s-image-swapper/issues/1146) [#1148](https://github.com/estahn/k8s-image-swapper/issues/1148) [#1118](https://github.com/estahn/k8s-image-swapper/issues/1118) [#1157](https://github.com/estahn/k8s-image-swapper/issues/1157) [#1146](https://github.com/estahn/k8s-image-swapper/issues/1146) [#1148](https://github.com/estahn/k8s-image-swapper/issues/1148) [#1118](https://github.com/estahn/k8s-image-swapper/issues/1118)
* **deps:** Bump github.com/prometheus/client_golang from 1.13.1 to 1.14.0 ([#392](#392)) ([af00594](af00594)), closes [#1150](https://github.com/estahn/k8s-image-swapper/issues/1150) [#1103](https://github.com/estahn/k8s-image-swapper/issues/1103) [prometheus/client_golang#1118](prometheus/client_golang#1118) [prometheus/client_golang#1103](prometheus/client_golang#1103) [prometheus/client_golang#1125](prometheus/client_golang#1125) [prometheus/client_golang#1130](prometheus/client_golang#1130) [prometheus/client_golang#1148](prometheus/client_golang#1148) [prometheus/client_golang#1146](prometheus/client_golang#1146) [prometheus/client_golang#1152](prometheus/client_golang#1152) [#1150](https://github.com/estahn/k8s-image-swapper/issues/1150) [#1103](https://github.com/estahn/k8s-image-swapper/issues/1103) [#1162](https://github.com/estahn/k8s-image-swapper/issues/1162) [#1161](https://github.com/estahn/k8s-image-swapper/issues/1161) [#1160](https://github.com/estahn/k8s-image-swapper/issues/1160) [#1136](https://github.com/estahn/k8s-image-swapper/issues/1136) [#1133](https://github.com/estahn/k8s-image-swapper/issues/1133) [#1150](https://github.com/estahn/k8s-image-swapper/issues/1150) [#1152](https://github.com/estahn/k8s-image-swapper/issues/1152)
* **deps:** Bump github.com/spf13/viper from 1.13.0 to 1.14.0 ([#385](#385)) ([6f79498](6f79498)), closes [spf13/viper#1457](spf13/viper#1457) [spf13/viper#1458](spf13/viper#1458) [spf13/viper#1460](spf13/viper#1460) [spf13/viper#1428](spf13/viper#1428) [spf13/viper#1406](spf13/viper#1406) [spf13/viper#1437](spf13/viper#1437) [spf13/viper#1453](spf13/viper#1453) [spf13/viper#1449](spf13/viper#1449) [spf13/viper#1461](spf13/viper#1461)
* **deps:** Bump golangci/golangci-lint-action from 3.3.0 to 3.3.1 ([#389](#389)) ([0b50f7b](0b50f7b)), closes [golangci/golangci-lint-action#590](golangci/golangci-lint-action#590) [golangci/golangci-lint-action#591](golangci/golangci-lint-action#591) [golangci/golangci-lint-action#592](golangci/golangci-lint-action#592) [golangci/golangci-lint-action#593](golangci/golangci-lint-action#593) [golangci/golangci-lint-action#594](golangci/golangci-lint-action#594) [golangci/golangci-lint-action#595](golangci/golangci-lint-action#595) [golangci/golangci-lint-action#596](golangci/golangci-lint-action#596) [golangci/golangci-lint-action#597](golangci/golangci-lint-action#597) [golangci/golangci-lint-action#598](golangci/golangci-lint-action#598) [golangci/golangci-lint-action#599](golangci/golangci-lint-action#599) [#599](#599) [#598](#598) [#596](#596) [#595](#595) [#593](#593) [#591](#591) [#590](#590)
* **deps:** Bump hmarr/auto-approve-action from 2 to 3 ([#396](#396)) ([0b982a2](0b982a2)), closes [hmarr/auto-approve-action#205](hmarr/auto-approve-action#205) [hmarr/auto-approve-action#202](hmarr/auto-approve-action#202) [hmarr/auto-approve-action#202](hmarr/auto-approve-action#202) [hmarr/auto-approve-action#200](hmarr/auto-approve-action#200) [hmarr/auto-approve-action#200](hmarr/auto-approve-action#200) [hmarr/auto-approve-action#186](hmarr/auto-approve-action#186) [hmarr/auto-approve-action#191](hmarr/auto-approve-action#191) [#210](#210) [#205](#205)
* **deps:** Bump k8s.io/api from 0.25.3 to 0.25.4 ([#401](#401)) ([0f80b5d](0f80b5d))
* **deps:** Bump k8s.io/apimachinery from 0.25.3 to 0.25.4 ([#399](#399)) ([1f0944f](1f0944f)), closes [#112218](https://github.com/estahn/k8s-image-swapper/issues/112218) [haoruan/automated-cherry-pick-of-#111936](https://github.com/haoruan/automated-cherry-pick-of-/issues/111936)
* **deps:** Bump k8s.io/client-go from 0.25.3 to 0.25.4 ([#400](#400)) ([ad036e0](ad036e0))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] - Stop func might not wait for job to finish
2 participants