Bug 1734009: Unidling: minimize iptables lock contention #23426

squeed · 2019-07-18T19:57:17Z

This PR includes

backport of upstream PR 71735, which moves the userspace proxy over to an asynchronous model
Changes needed to use that backport
A small carry patch that lets us tweak the proxy internals slightly
A patch that combines both proxy's sync loops, preventing lock contention

Fixes: BZ 1699341

openshift-ci-robot · 2019-07-18T19:57:21Z

@squeed: This pull request references a valid Bugzilla bug. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

Bug 1699341: Unidling: minimize iptables lock contention

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.

squeed · 2019-07-18T19:57:25Z

@dcbw PTAL, thanks!

squeed · 2019-07-18T20:21:14Z

Uh, unit test failures have nothing to do with this. Is origin broken?

danwinship · 2019-07-18T20:49:47Z

/hold
This needs to be fixed in master and 4.1 before it can be fixed in 3.11

squeed · 2019-07-18T21:06:42Z

This needs to be fixed in master and 4.1 before it can be fixed in 3.11

Yup. I just picked up 3.11 first, since there already was the backport and the sdn repo was being migrated as I started work.

dcbw · 2019-07-18T21:15:50Z

@squeed are you sure you want the hybrid-proxy: bump debug level commit? I believe that was just for local debugging to make sure unidling was working.

dcbw · 2019-07-18T21:17:40Z

@squeed could also squash these two together:

UPSTREAM: 71735: proxy/userspace: respect minSyncInterval
proxy: consolidate ServicesHandler/EndpointsHandler into ProxyProvider

vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier_openshift.go

dcbw · 2019-07-18T21:22:59Z

otherwise looks good to me

Proxies should be able to cleanly figure out when endpoints have been synced, so make all ProxyProviders also implement EndpointsHandler and pass those through to loadbalancers when required.

openshift-ci-robot · 2019-07-29T12:41:59Z

@squeed: This pull request references an invalid Bugzilla bug:

expected dependent Bugzilla bug to be in one of the following states: VERIFIED, RELEASE_PENDING, CLOSED (ERRATA), but it is POST instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

Bug 1734009: Unidling: minimize iptables lock contention

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.

This backports openshift/sdn#8 Both proxies have an asynchronous apply loop, but we really need them to apply sequentially. So, run them as part of the same runner.

nstielau · 2019-08-06T23:10:50Z

Are we good to merge this so we can get it in August 12th 3.11.z?

squeed · 2019-08-08T14:33:36Z

/bugzilla refresh

openshift-ci-robot · 2019-08-08T14:33:41Z

@squeed: This pull request references a valid Bugzilla bug. The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/bugzilla refresh

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.

squeed · 2019-08-08T15:39:55Z

/hold cancel

bparees · 2019-08-08T16:31:40Z

/approve

dcbw · 2019-08-20T21:20:54Z

pkg/network/proxy/proxy.go

+}
+
+func (proxy *OsdnProxy) OnServiceAdd(service *kapi.Service) {
+	glog.V(2).Infof("sdn proxy: add svc %s/%s: %v", service.Namespace, service.Name, service)


@squeed I guess this is a leftover from my original patches.

dcbw · 2019-08-26T17:16:33Z

/lgtm

openshift-ci-robot · 2019-08-26T17:19:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bparees, dcbw, squeed

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:

~~OWNERS~~ [bparees]

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

openshift-ci-robot · 2019-08-26T19:55:54Z

@squeed: All pull requests linked via external trackers have merged. Bugzilla bug 1734009 has been moved to the MODIFIED state.

In response to this:

Bug 1734009: Unidling: minimize iptables lock contention

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.

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Jul 18, 2019

openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Jul 18, 2019

openshift-ci-robot requested review from danwinship and dcbw July 18, 2019 19:57

openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 18, 2019

dcbw reviewed Jul 18, 2019

View reviewed changes

vendor/k8s.io/kubernetes/pkg/proxy/iptables/proxier_openshift.go Outdated Show resolved Hide resolved

squeed mentioned this pull request Jul 19, 2019

Unidling: minimize iptables lock contention openshift/sdn#8

Merged

openshift-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 26, 2019

proxy: consolidate ServicesHandler/EndpointsHandler into ProxyProvider

ba4ce14

Proxies should be able to cleanly figure out when endpoints have been synced, so make all ProxyProviders also implement EndpointsHandler and pass those through to loadbalancers when required.

squeed changed the title ~~Bug 1699341: Unidling: minimize iptables lock contention~~ Bug 1734009: Unidling: minimize iptables lock contention Jul 29, 2019

openshift-ci-robot added bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. and removed bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. labels Jul 29, 2019

dcbw and others added 4 commits July 29, 2019 18:30

sdn-proxy: update for combined service/endpoints handler

4c2ef93

UPSTREAM: 71735: proxy/userspace: respect minSyncInterval

c152340

UPSTREAM: <carry>: Allow low-level tweaking of proxy sync flow

204914c

proxy: Consolidate proxy apply loops.

4aadf91

This backports openshift/sdn#8 Both proxies have an asynchronous apply loop, but we really need them to apply sequentially. So, run them as part of the same runner.

openshift-ci-robot added the bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. label Aug 8, 2019

openshift-ci-robot removed the bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. label Aug 8, 2019

openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Aug 8, 2019

openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 8, 2019

dcbw reviewed Aug 20, 2019

View reviewed changes

openshift-ci-robot assigned dcbw Aug 26, 2019

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2019

openshift-merge-robot merged commit 30ff1fa into openshift:release-3.11 Aug 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1734009: Unidling: minimize iptables lock contention #23426

Bug 1734009: Unidling: minimize iptables lock contention #23426

squeed commented Jul 18, 2019

openshift-ci-robot commented Jul 18, 2019

squeed commented Jul 18, 2019

squeed commented Jul 18, 2019

danwinship commented Jul 18, 2019

squeed commented Jul 18, 2019

dcbw commented Jul 18, 2019

dcbw commented Jul 18, 2019

dcbw commented Jul 18, 2019

openshift-ci-robot commented Jul 29, 2019

nstielau commented Aug 6, 2019

squeed commented Aug 8, 2019

openshift-ci-robot commented Aug 8, 2019

squeed commented Aug 8, 2019

bparees commented Aug 8, 2019

dcbw Aug 20, 2019

dcbw commented Aug 26, 2019

openshift-ci-robot commented Aug 26, 2019

openshift-ci-robot commented Aug 26, 2019

Bug 1734009: Unidling: minimize iptables lock contention #23426

Bug 1734009: Unidling: minimize iptables lock contention #23426

Conversation

squeed commented Jul 18, 2019

openshift-ci-robot commented Jul 18, 2019

squeed commented Jul 18, 2019

squeed commented Jul 18, 2019

danwinship commented Jul 18, 2019

squeed commented Jul 18, 2019

dcbw commented Jul 18, 2019

dcbw commented Jul 18, 2019

dcbw commented Jul 18, 2019

openshift-ci-robot commented Jul 29, 2019

nstielau commented Aug 6, 2019

squeed commented Aug 8, 2019

openshift-ci-robot commented Aug 8, 2019

squeed commented Aug 8, 2019

bparees commented Aug 8, 2019

dcbw Aug 20, 2019

Choose a reason for hiding this comment

dcbw commented Aug 26, 2019

openshift-ci-robot commented Aug 26, 2019

openshift-ci-robot commented Aug 26, 2019