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

Env var to control downloading to temp path #1667

Merged
merged 2 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cmd/zt_interceptors_for_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ type mockedLifecycleManager struct {
outputFormat common.OutputFormat
}

func (m *mockedLifecycleManager) DownloadToTempPath() bool {
return false
}

func (m *mockedLifecycleManager) Progress(o common.OutputBuilder) {
select {
case m.progressLog <- o(common.EOutputFormat.Text()):
Expand Down
8 changes: 8 additions & 0 deletions common/environment.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,3 +361,11 @@ func (EnvironmentVariable) MimeMapping() EnvironmentVariable {
Description: "Location of the file to override default OS mime mapping",
}
}

func (EnvironmentVariable) DownloadToTempPath() EnvironmentVariable {
return EnvironmentVariable {
Name: "AZCOPY_DOWNLOAD_TO_TEMP_PATH",
DefaultValue: "true",
Description: "Configures azcopy to download to a temp path before actual download. Allowed values are true/false",
}
}
10 changes: 10 additions & 0 deletions common/lifecyleMgr.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ type LifecycleMgr interface {
RegisterCloseFunc(func())
SetForceLogging()
IsForceLoggingDisabled() bool
DownloadToTempPath() bool
}

func GetLifecycleMgr() LifecycleMgr {
Expand Down Expand Up @@ -590,6 +591,15 @@ func (lcm *lifecycleMgr) IsForceLoggingDisabled() bool {
return lcm.disableSyslog
}

func (lcm *lifecycleMgr) DownloadToTempPath() bool {
ret, err := strconv.ParseBool(lcm.GetEnvironmentVariable(EEnvironmentVariable.DownloadToTempPath()))
Copy link
Contributor

Choose a reason for hiding this comment

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

consider doing what SetForceLogging does above, and save the parsed result rather than parsing it every time.

if err != nil {
// By default we'll download to temp path
ret = true
}
return ret
}

// captures the common logic of exiting if there's an expected error
func PanicIfErr(err error) {
if err != nil {
Expand Down
30 changes: 18 additions & 12 deletions ste/xfer-remoteToLocal-file.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func remoteToLocal_file(jptm IJobPartTransferMgr, p pipeline.Pipeline, pacer pac
// to correct name.
pseudoId := common.NewPseudoChunkIDForWholeFile(info.Source)
jptm.LogChunkStatus(pseudoId, common.EWaitReason.CreateLocalFile())
dstFile, err = createDestinationFile(jptm, info.getTempDownloadPath(), fileSize, writeThrough)
dstFile, err = createDestinationFile(jptm, info.getDownloadPath(), fileSize, writeThrough)
mohsha-msft marked this conversation as resolved.
Show resolved Hide resolved
jptm.LogChunkStatus(pseudoId, common.EWaitReason.ChunkDone()) // normal setting to done doesn't apply to these pseudo ids
if err != nil {
failFileCreation(err)
Expand Down Expand Up @@ -342,7 +342,7 @@ func epilogueWithCleanupDownload(jptm IJobPartTransferMgr, dl downloader, active

// check length if enabled (except for dev null and decompression case, where that's impossible)
if info.DestLengthValidation && info.Destination != common.Dev_Null && !jptm.ShouldDecompress() {
fi, err := common.OSStat(info.getTempDownloadPath())
fi, err := common.OSStat(info.getDownloadPath())

if err != nil {
jptm.FailActiveDownload("Download length check", err)
Expand All @@ -351,14 +351,16 @@ func epilogueWithCleanupDownload(jptm IJobPartTransferMgr, dl downloader, active
}
}

//Rename back to original name. At this point, we're sure the file is completely
//check if we need to rename back to original name. At this point, we're sure the file is completely
//downloaded and not corrupt. Infact, post this point we should only log errors and
//not fail the transfer.
if err == nil && !strings.EqualFold(info.Destination, common.Dev_Null) {
renameErr := os.Rename(info.getTempDownloadPath(), info.Destination)
renameNecessary := !strings.EqualFold(info.getDownloadPath(), info.Destination) &&
!strings.EqualFold(info.Destination, common.Dev_Null)
if err == nil && renameNecessary {
renameErr := os.Rename(info.getDownloadPath(), info.Destination)
if renameErr != nil {
jptm.LogError(info.Destination, fmt.Sprintf(
"Failed to rename. File at %s", info.getTempDownloadPath()), renameErr)
"Failed to rename. File at %s", info.getDownloadPath()), renameErr)
}
}
}
Expand Down Expand Up @@ -461,19 +463,23 @@ func tryDeleteFile(info TransferInfo, jptm IJobPartTransferMgr) {
return
}

err := deleteFile(info.getTempDownloadPath())
err := deleteFile(info.getDownloadPath())
if err != nil {
// If there was an error deleting the file, log the error
jptm.LogError(info.Destination, "Delete File Error ", err)
}
}

//Returns the path of temp file to be downloaded. The paths is in format
// Returns the path of file to be downloaded. If we want to
// download to a temp path we return a temp paht in format
// /actual/parent/path/.azDownload-<jobID>-<actualFileName>
func (info *TransferInfo) getTempDownloadPath() string {
parent, fileName := filepath.Split(info.Destination)
fileName = fmt.Sprintf(azcopyTempDownloadPrefix, info.JobID.String()) + fileName
return filepath.Join(parent, fileName)
func (info *TransferInfo) getDownloadPath() string {
if common.GetLifecycleMgr().DownloadToTempPath() {
parent, fileName := filepath.Split(info.Destination)
fileName = fmt.Sprintf(azcopyTempDownloadPrefix, info.JobID.String()) + fileName
return filepath.Join(parent, fileName)
}
return info.Destination
}

// conforms to io.Writer and io.Closer
Expand Down