Project

Profile

Help

Issue #2689

closed

Don't use ssh connection sharing in rsync distributor

Added by rmcgover about 7 years ago. Updated almost 5 years ago.

Status:
CLOSED - CURRENTRELEASE
Priority:
Normal
Category:
-
Sprint/Milestone:
-
Start date:
Due date:
Estimated time:
Severity:
2. Medium
Version:
2.12.2
Platform Release:
2.13.1
OS:
Triaged:
Yes
Groomed:
Yes
Sprint Candidate:
No
Tags:
Easy Fix, Pulp 2
Sprint:
Sprint 19
Quarter:

Description

In pulp platform, server/pulp/plugins/rsync/publish.py , ssh for rsync distributor is assembled like this:

cmd = ['ssh', '-l', user]
key = self.get_config().flatten()["remote"]['ssh_identity_file']
cmd += ['-i', key,
        '-o', 'StrictHostKeyChecking no',
        '-o', 'UserKnownHostsFile /dev/null',
        '-S', '/tmp/rsync_distributor-%r@%h:%p',
        '-o', 'ControlMaster auto',
        '-o', 'ControlPersist 10']

Can the unconditional use of ControlMaster please be removed?

The usage of ControlMaster=auto here causes the following problem: if an ssh command is the ControlMaster, it won't exit until all clients using that control socket has exited.

Consider the following scenario to understand why this is an issue:

- there's an idle Pulp with 8 celery workers on one host
- many repo rsync publishes are enqueued
- worker 1 starts publishing with rsync distributor; when it starts rsync, it becomes the ControlMaster
- worker 2 through 8 start publishing other repos & connect to the ControlMaster
- worker 1 upload finishes, but ssh can't exit until the control socket is idle
- worker 1's task can't complete until workers 2 through 8 are all idle at the same time which is unlikely to happen until the entire queue of rsync publishes has completed

Overall, the impact of this is that one worker's rsync publish can unpredictably block completion on the publish from other workers, leading to less throughput due to resources being locked longer than they should, and misleading repo publish times.

I note that if the options relating to ControlMaster here were removed, it would still be possible for Pulp users to enable the feature by editing the ssh_config on the system where Pulp is deployed, if they have a use-case for that.

Actions #2

Updated by mhrivnak about 7 years ago

Just to confirm, is this all you're hoping to see?

diff --git a/server/pulp/plugins/rsync/publish.py b/server/pulp/plugins/rsync/publish.py
index 331eb42..d0da3cd 100644
--- a/server/pulp/plugins/rsync/publish.py
+++ b/server/pulp/plugins/rsync/publish.py
@@ -103,9 +103,7 @@ class RSyncPublishStep(PublishStep):
         cmd += ['-i', key,
                 '-o', 'StrictHostKeyChecking no',
                 '-o', 'UserKnownHostsFile /dev/null',
-                '-S', '/tmp/rsync_distributor-%r@%h:%p',
-                '-o', 'ControlMaster auto',
-                '-o', 'ControlPersist 10']
+                '-S', '/tmp/rsync_distributor-%r@%h:%p']
         if args:
             cmd += args
         return cmd
Actions #3

Updated by rmcgover about 7 years ago

Yeah, that would be sufficient, (I believe the -S may as well be removed too, but would be harmless to leave it.)

Actions #4

Updated by ttereshc about 7 years ago

  • Triaged changed from No to Yes
  • Tags Easy Fix added
Actions #5

Updated by bmbouter about 7 years ago

  • Groomed changed from No to Yes
Actions #6

Updated by mhrivnak about 7 years ago

  • Sprint/Milestone set to 37
Actions #7

Updated by ipanova@redhat.com about 7 years ago

  • Status changed from NEW to ASSIGNED
  • Assignee set to ipanova@redhat.com

Added by ipanova@redhat.com about 7 years ago

Revision 80ff8c8a | View on GitHub

Don't use ssh connection sharing in rsync distributor.

closes #2689 https://pulp.plan.io/issues/2689

Actions #8

Updated by ipanova@redhat.com about 7 years ago

  • Status changed from ASSIGNED to POST
Actions #9

Updated by jortel@redhat.com almost 7 years ago

  • Sprint/Milestone changed from 37 to 38
Actions #10

Updated by ipanova@redhat.com almost 7 years ago

  • Status changed from POST to MODIFIED
Actions #11

Updated by bmbouter almost 7 years ago

  • Tags RCM added
Actions #12

Updated by bizhang almost 7 years ago

  • Platform Release set to 2.13.1
Actions #13

Updated by bizhang almost 7 years ago

  • Status changed from MODIFIED to 5
Actions #14

Updated by bizhang almost 7 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE
Actions #15

Updated by bmbouter about 6 years ago

  • Sprint set to Sprint 19
Actions #16

Updated by bmbouter about 6 years ago

  • Sprint/Milestone deleted (38)
Actions #17

Updated by bmbouter about 5 years ago

  • Tags Pulp 2 added
Actions #18

Updated by bmbouter almost 5 years ago

  • Tags deleted (RCM)

Also available in: Atom PDF