[ARVADOS] updated: 1.1.0-103-gf830a94
Git user
git at public.curoverse.com
Tue Nov 7 11:01:33 EST 2017
Summary of changes:
services/crunch-run/crunchrun.go | 52 ++++++++++++-----------------------
services/crunch-run/crunchrun_test.go | 13 ++++++++-
2 files changed, 30 insertions(+), 35 deletions(-)
via f830a9450430422f6aaf6ec36bb760db530f4376 (commit)
from 8d60bb5beda34d3837b4c9917576187abdbf986b (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit f830a9450430422f6aaf6ec36bb760db530f4376
Author: Peter Amstutz <pamstutz at veritasgenetics.com>
Date: Tue Nov 7 10:14:40 2017 -0500
12538: Refactor crunch-run shutdown
Arvados-DCO-1.1-Signed-off-by: Peter Amstutz <pamstutz at veritasgenetics.com>
diff --git a/services/crunch-run/crunchrun.go b/services/crunch-run/crunchrun.go
index 557f30d..ead9184 100644
--- a/services/crunch-run/crunchrun.go
+++ b/services/crunch-run/crunchrun.go
@@ -221,7 +221,7 @@ func (runner *ContainerRunner) stop() {
}
}
-func (runner *ContainerRunner) teardown() {
+func (runner *ContainerRunner) stopSignals() {
if runner.SigChan != nil {
signal.Stop(runner.SigChan)
close(runner.SigChan)
@@ -358,8 +358,6 @@ func (runner *ContainerRunner) SetupMounts() (err error) {
return fmt.Errorf("While creating keep mount temp dir: %v", err)
}
- runner.CleanupTempDir = append(runner.CleanupTempDir, runner.ArvMountPoint)
-
pdhOnly := true
tmpcount := 0
arvMountCmd := []string{
@@ -1255,15 +1253,15 @@ func (runner *ContainerRunner) getCollectionManifestForPath(mnt arvados.Mount, b
func (runner *ContainerRunner) CleanupDirs() {
if runner.ArvMount != nil {
- delay := 8
+ var delay int64 = 8
umount := exec.Command("arv-mount", fmt.Sprintf("--unmount-timeout=%d", delay), "--unmount", runner.ArvMountPoint)
umnterr := umount.Start()
if umnterr != nil {
runner.CrunchLog.Printf("Error running %v: %v", umount.Args, umnterr)
} else {
// If arv-mount --unmount gets stuck for any reason, we
- // don't want to wait for it. Spin off the wait for
- // child process so it doesn't block crunch-run.
+ // don't want to wait for it forever. Do Wait() in a goroutine
+ // so it doesn't block crunch-run.
go func() {
mnterr := umount.Wait()
if mnterr != nil {
@@ -1271,23 +1269,23 @@ func (runner *ContainerRunner) CleanupDirs() {
}
}()
- timeout := time.NewTimer((delay + 1) * time.Second)
select {
case <-runner.ArvMountExit:
break
- case <-timeout.C:
+ case <-time.After(time.Duration((delay + 1) * int64(time.Second))):
runner.CrunchLog.Printf("Timed out waiting for %v", umount.Args)
+ umount.Process.Kill()
}
}
+ }
+
+ if runner.ArvMountPoint != "" {
if rmerr := os.Remove(runner.ArvMountPoint); rmerr != nil {
runner.CrunchLog.Printf("While cleaning up arv-mount directory %s: %v", runner.ArvMountPoint, rmerr)
}
}
for _, tmpdir := range runner.CleanupTempDir {
- if tmpdir == runner.ArvMountPoint {
- continue
- }
if rmerr := os.RemoveAll(tmpdir); rmerr != nil {
runner.CrunchLog.Printf("While cleaning up temporary directory %s: %v", tmpdir, rmerr)
}
@@ -1414,13 +1412,6 @@ func (runner *ContainerRunner) Run() (err error) {
runner.CrunchLog.Printf("Executing on host '%s'", hostname)
}
- // Clean up temporary directories _after_ finalizing
- // everything (if we've made any by then)
- defer func() {
- runner.CrunchLog.Printf("crunch-run finished")
- }()
- defer runner.CleanupDirs()
-
runner.finalState = "Queued"
defer func() {
@@ -1446,28 +1437,21 @@ func (runner *ContainerRunner) Run() (err error) {
// Log the error encountered in Run(), if any
checkErr(err)
- if runner.finalState == "Queued" {
- runner.CrunchLog.Close()
- runner.UpdateContainerFinal()
- return
- }
-
- if runner.IsCancelled() {
- runner.finalState = "Cancelled"
- // but don't return yet -- we still want to
- // capture partial output and write logs
+ if runner.finalState != "Queued" {
+ if runner.IsCancelled() {
+ runner.finalState = "Cancelled"
+ }
+ checkErr(runner.CaptureOutput())
}
- checkErr(runner.CaptureOutput())
checkErr(runner.CommitLogs())
checkErr(runner.UpdateContainerFinal())
- // The real log is already closed, but then we opened
- // a new one in case we needed to log anything while
- // finalizing.
- runner.CrunchLog.Close()
+ runner.stopSignals()
+ runner.CleanupDirs()
- runner.teardown()
+ runner.CrunchLog.Printf("crunch-run finished")
+ runner.CrunchLog.Close()
}()
err = runner.fetchContainerRecord()
diff --git a/services/crunch-run/crunchrun_test.go b/services/crunch-run/crunchrun_test.go
index 5992be0..bc0b312 100644
--- a/services/crunch-run/crunchrun_test.go
+++ b/services/crunch-run/crunchrun_test.go
@@ -670,7 +670,7 @@ func FullRunHelper(c *C, record string, extraMounts []string, exitCode int, fn f
}
c.Check(api.WasSetRunning, Equals, true)
- c.Check(api.Content[api.Calls-1]["container"].(arvadosclient.Dict)["log"], NotNil)
+ c.Check(api.Content[api.Calls-2]["container"].(arvadosclient.Dict)["log"], NotNil)
if err != nil {
for k, v := range api.Logs {
@@ -1010,6 +1010,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
"--read-write", "--crunchstat-interval=5",
"--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/tmp"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1028,6 +1029,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
"--read-write", "--crunchstat-interval=5",
"--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/out", realTemp + "/3:/tmp"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1048,6 +1050,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
"--read-write", "--crunchstat-interval=5",
"--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/tmp", stubCertPath + ":/etc/arvados/ca-certificates.crt:ro"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
@@ -1070,6 +1073,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
"--read-write", "--crunchstat-interval=5",
"--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/tmp0:/keeptmp"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1094,6 +1098,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
sort.StringSlice(cr.Binds).Sort()
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53:/keepinp:ro",
realTemp + "/keep1/tmp0:/keepout"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1119,6 +1124,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
sort.StringSlice(cr.Binds).Sort()
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/keep1/by_id/59389a8f9ee9d399be35462a0f92541c+53:/keepinp:ro",
realTemp + "/keep1/tmp0:/keepout"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1143,6 +1149,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
content, err := ioutil.ReadFile(realTemp + "/2/mountdata.json")
c.Check(err, IsNil)
c.Check(content, DeepEquals, []byte(test.out))
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1166,6 +1173,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
"--read-write", "--crunchstat-interval=5",
"--file-cache", "512", "--mount-tmp", "tmp0", "--mount-by-pdh", "by_id", realTemp + "/keep1"})
c.Check(cr.Binds, DeepEquals, []string{realTemp + "/2:/tmp", realTemp + "/keep1/tmp0:/tmp/foo:ro"})
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1184,6 +1192,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, NotNil)
c.Check(err, ErrorMatches, `Writable mount points are not permitted underneath the output_path.*`)
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1202,6 +1211,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, NotNil)
c.Check(err, ErrorMatches, `Only mount points of kind 'collection' are supported underneath the output_path.*`)
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
@@ -1218,6 +1228,7 @@ func (s *TestSuite) TestSetupMounts(c *C) {
err := cr.SetupMounts()
c.Check(err, NotNil)
c.Check(err, ErrorMatches, `Unsupported mount kind 'tmp' for stdin.*`)
+ os.RemoveAll(cr.ArvMountPoint)
cr.CleanupDirs()
checkEmpty()
}
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list