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

Remove unnecessary txn watcher started channel/topic #13312

Merged
merged 1 commit into from
Sep 7, 2021

Conversation

wallyworld
Copy link
Member

As seen in the linked bug, there can be a panic if the txn watcher worker bounces and closes a "watcherStarted" channel twice. However, that channel and associated pubsub topic is unnecessary in the first place, so this PR removes them both.

QA steps

bootstrap and deploy a couple of charms and relate

Bug reference

https://bugs.launchpad.net/juju/+bug/1942421

@wallyworld
Copy link
Member Author

migration critical bug

@@ -181,9 +177,6 @@ func OpenStatePool(args OpenParams) (_ *StatePool, err error) {
RestartDelay: time.Second,
Clock: args.Clock,
})
pool.hubUnsubFn = pool.hub.Subscribe(watcher.TxnWatcherStarting, func(string, interface{}) {
Copy link
Member

Choose a reason for hiding this comment

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

I suspect this was caused by the fact we weren't catching every location that the unsubscribe could have been called. Either way, I'm happy that we've removed this, as it's not used. Seems like originally it was created because they could.

@wallyworld
Copy link
Member Author

$$merge$$

@jujubot jujubot merged commit db44622 into juju:2.9 Sep 7, 2021
@achilleasa achilleasa mentioned this pull request Sep 13, 2021
jujubot added a commit that referenced this pull request Sep 13, 2021
#13329

This PR forward ports 2.9 into develop. The following PRs are included in this port:
 - Merge pull request #13324 from achilleasa/2.9-add-ovs-integration-test
 - Merge pull request #13328 from manadart/2.9-assess-series-upgrade
 - Merge pull request #13327 from wallyworld/azure-tests-fix
 - Merge pull request #13325 from SimonRichardson/raft-worker-errors
 - Merge pull request #13274 from SimonRichardson/lxd-network-devices-config-host-name
 - Merge pull request #13323 from jujubot/increment-to-2.9.15
 - Merge pull request #13321 from wallyworld/more-secret-metadata
 - Merge pull request #13319 from manadart/2.9-bridge-policy
 - Merge pull request #13320 from SimonRichardson/revert-lxd-changes
 - Merge pull request #13194 from juanmanuel-tirado/patch-1
 - Merge pull request #13318 from hpidcock/fix-1942948
 - Merge pull request #13314 from simondeziel/snap-ack
 - Merge pull request #13297 from achilleasa/2.9-allow-empty-openvswitch-blocks-in-netplan-config
 - Merge pull request #13317 from jujubot/increment-to-2.9.14
 - Merge pull request #13316 from hpidcock/fix-1942948
 - Merge pull request #13296 from ycliuhw/fix/registry-oauth2
 - Merge pull request #13311 from wallyworld/unitagent-missing-charm
 - Merge pull request #13315 from wallyworld/lxd-not-found-fix
 - Merge pull request #13221 from juanmanuel-tirado/status_watch_flag
 - Merge pull request #13312 from wallyworld/remove-txnwatcher-started
 - Merge pull request #13309 from kot0dama/fix-instrospection-posix-shell-2.9

The following files had merge conflicts that had to be resolved (please double-check the changes in last commit):
- caas/kubernetes/provider/bootstrap_test.go
- feature/flags.go
- scripts/win-installer/setup.iss
- snap/snapcraft.yaml
- state/pool.go
- version/version.go
- worker/uniter/relation/state_test.go
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants