Project

Profile

Help

Issue #2689

Don't use ssh connection sharing in rsync distributor

Added by rmcgover almost 5 years ago. Updated over 2 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.

Associated revisions

Revision 80ff8c8a View on GitHub
Added by ipanova@redhat.com almost 5 years ago

Don't use ssh connection sharing in rsync distributor.

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

History

#2 Updated by mhrivnak almost 5 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

#3 Updated by rmcgover almost 5 years ago

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

#4 Updated by ttereshc almost 5 years ago

  • Triaged changed from No to Yes
  • Tags Easy Fix added

#5 Updated by bmbouter almost 5 years ago

  • Groomed changed from No to Yes

#6 Updated by mhrivnak almost 5 years ago

  • Sprint/Milestone set to 37

#7 Updated by ipanova@redhat.com almost 5 years ago

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

#8 Updated by ipanova@redhat.com almost 5 years ago

  • Status changed from ASSIGNED to POST

#9 Updated by jortel@redhat.com over 4 years ago

  • Sprint/Milestone changed from 37 to 38

#10 Updated by ipanova@redhat.com over 4 years ago

  • Status changed from POST to MODIFIED

#11 Updated by bmbouter over 4 years ago

  • Tags RCM added

#12 Updated by bizhang over 4 years ago

  • Platform Release set to 2.13.1

#13 Updated by bizhang over 4 years ago

  • Status changed from MODIFIED to 5

#14 Updated by bizhang over 4 years ago

  • Status changed from 5 to CLOSED - CURRENTRELEASE

#15 Updated by bmbouter almost 4 years ago

  • Sprint set to Sprint 19

#16 Updated by bmbouter almost 4 years ago

  • Sprint/Milestone deleted (38)

#17 Updated by bmbouter almost 3 years ago

  • Tags Pulp 2 added

#18 Updated by bmbouter over 2 years ago

  • Tags deleted (RCM)

Also available in: Atom PDF