Skip to content

Commit

Permalink
Truncate the plugin output file as soon as possible
Browse files Browse the repository at this point in the history
Additionally, ensure we fsync the final output file when we're finished.

Fixes #519
Fixes #528
  • Loading branch information
bbockelm committed Dec 21, 2023
1 parent b27a6ed commit 8fc839d
Showing 1 changed file with 37 additions and 13 deletions.
50 changes: 37 additions & 13 deletions cmd/plugin.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,12 @@ package main
import (
"bufio"
"fmt"
"io/fs"
"net/url"
"os"
"path/filepath"
"strings"
"syscall"
"time"

"github.com/pelicanplatform/pelican/classads"
Expand Down Expand Up @@ -139,7 +141,8 @@ func stashPluginMain(args []string) {
if getCaches {
urls, err := client.GetCacheHostnames(testCachePath)
if err != nil {
log.Panicln("Failed to get cache URLs:", err)
log.Errorln("Failed to get cache URLs:", err)
os.Exit(1)
}

cachesToTry := client.CachesToTry
Expand Down Expand Up @@ -168,7 +171,8 @@ func stashPluginMain(args []string) {
// Open the input and output files
infileFile, err := os.Open(infile)
if err != nil {
log.Panicln("Failed to open infile:", err)
log.Errorln("Failed to open infile:", err)
os.Exit(1)
}
defer infileFile.Close()
// Read in classad from stdin
Expand All @@ -185,6 +189,25 @@ func stashPluginMain(args []string) {
}
}

// NOTE: HTCondor 23.3.0 and before would reuse the outfile names for multiple
// transfers, meaning the results of prior plugin invocations would be present
// by default in the outfile. Combined with a bug that considered any exit code
// besides `1` a success (note: a go panic is exit code `2`), this caused the starter
// to incorrectly interpret plugin failures as successes, potentially leaving the user
// with missing or truncated output files.
//
// By moving the truncation of the output file to a very early codepath, we reduce
// the chances of hitting this problem.
outputFile := os.Stdout
if useOutFile {
var err error
outputFile, err = os.Create(outfile)
if err != nil {
log.Errorln("Failed to open outfile:", err)
}
defer outputFile.Close()
}

var resultAds []*classads.ClassAd
retryable := false
for _, transfer := range transfers {
Expand Down Expand Up @@ -259,21 +282,12 @@ func stashPluginMain(args []string) {

}

outputFile := os.Stdout
if useOutFile {
var err error
outputFile, err = os.Create(outfile)
if err != nil {
log.Panicln("Failed to open outfile:", err)
}
defer outputFile.Close()
}

success := true
for _, resultAd := range resultAds {
_, err := outputFile.WriteString(resultAd.String() + "\n")
if err != nil {
log.Panicln("Failed to write to outfile:", err)
log.Errorln("Failed to write to outfile:", err)
os.Exit(1)
}
transferSuccess, err := resultAd.Get("TransferSuccess")
if err != nil {
Expand All @@ -282,6 +296,16 @@ func stashPluginMain(args []string) {
}
success = success && transferSuccess.(bool)
}
if err = outputFile.Sync(); err != nil {
var perr *fs.PathError
var serr syscall.Errno
if errors.As(err, &perr) && errors.As(perr.Unwrap(), &serr) && serr == syscall.EINVAL {
log.Debugf("Error when syncing: %s; can be ignored\n", perr)
} else {
log.Errorln("Failed to sync output file:", err)
os.Exit(1)
}
}

if success {
os.Exit(0)
Expand Down

0 comments on commit 8fc839d

Please sign in to comment.