roachprod: change scp flags to avoid copy via local host #99283
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously default scp option made it to copy files via local host. This was causing unnecessary delays and network usage because files were first copied to one node, then distributed from that node using scp between remote locations. Unfortunately the default scp behaviour is to fetch from src and put to dst to avoid authenticating from src to dst. This commit adds scp options to do direct copy. Fixes: cockroachdb#99103 Release note: None
aliher1911
requested review from
herkolategan and
smg260
and removed request for
a team
|
This change is |
|
Without fix put to 6 nodes takes 4:30: With this fix put takes 0:58: |
tbg
approved these changes
Mar 23, 2023
|
bors r=tbg |
|
Build succeeded: |
|
@aliher1911 seems like We can conditionally add that flag only on OSX/BSD; that should help you with your development flow while keeping behavior on TC as it was before. |
|
Argh I was suspicious that its not universal and too trigger happy! |
|
Revert PR: #99419. |
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Mar 23, 2023
In cockroachdb#99283 the flags `-R -A` were added to the `scp` command invocation, in order to ensure that copies between two remote hosts are done without transferring to the localhost. These flags are only supported on MacOS (`"darwin"`) and not on Linux, as this behavior is already the default on Linux. This change fixes that issue by only using these flags on MacOS, allowing the `scp` invocation (and by extension, roachprod) to work on Linux. Epic: none Release note: None
AlexTalks
added a commit
to AlexTalks/cockroach
that referenced
this pull request
Apr 3, 2023
In cockroachdb#99283 the flags `-R -A` were added to the `scp` command invocation, in order to ensure that copies between two remote hosts are done without transferring to the localhost. These flags are only supported on MacOS (`"darwin"`) and not on Linux, as this behavior is already the default on Linux. This change fixes that issue by only using these flags on MacOS, allowing the `scp` invocation (and by extension, roachprod) to work on Linux. Epic: none Release note: None
craig bot
pushed a commit
that referenced
this pull request
Apr 3, 2023
99430: roachprod: ensure scp uses supported flags r=AlexTalks a=AlexTalks In #99283 the flags `-R -A` were added to the `scp` command invocation, in order to ensure that copies between two remote hosts are done without transferring to the localhost. These flags are only supported on MacOS (`"darwin"`) and not on Linux, as this behavior is already the default on Linux. This change fixes that issue by only using these flags on MacOS, allowing the `scp` invocation (and by extension, roachprod) to work on Linux. Epic: none Release note: None 99982: clusterversion: clarify where to add new version gates during stability r=rail a=celiala The previous direction said to always put append version gates to the end of the list. However, during the stability period, before we mint the release branch, there are two places to add new gates: - new gates should go _before_ the "mint" version, if it is being backported to the release branch and we have not yet merged the "mint" version to the release branch. - new gates for the upcoming development, should be appended to the end of the list. This direction should be reverted/simplified (to direct to always append) once the commit to "mint" 23.1 has been backported and merged to release-23.1. Tracking issue for this is: https://cockroachlabs.atlassian.net/browse/REL-311 Release note: None Epic: REL-283 100378: kvserver: fix merge queue test failure due to race r=AlexTalks a=AlexTalks Previously, we saw `TestMergeQueueDoesNotInterruptReplicationChange` fail on arm64 machines (#99349), which, after testing, was determined to be due to timing issues in the test. As such, this change modifies the test so as to not assume that the snapshot will start within the 100ms time frame used as a buffer, and will instead wait for the snapshot to start before attempting the merge. Fixes: #99349 Release note: None Co-authored-by: Alex Sarkesian <[email protected]> Co-authored-by: Celia La <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously default scp option made it to copy files via local host. This was causing unnecessary delays and network usage because files were first copied to one node, then distributed from that node using scp between remote locations. Unfortunately the default scp behaviour is to fetch from src and put to dst to avoid authenticating from src to dst.
This commit adds scp options to do direct copy.
Fixes: #99103
Release note: None