Skip to content

Commit

Permalink
Serially delete directories and files in build results.
Browse files Browse the repository at this point in the history
Previously, this was deleted asynchronously. While this sounds
okay, it actually causes issue at scale with the number of
file descriptors that can be open at a given time for builds
with large numbers of atoms.
  • Loading branch information
TJ Lee committed Mar 28, 2016
1 parent bdbd4e1 commit ca2408a
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion app/master/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from enum import Enum
import os
from queue import Queue, Empty
import shutil
from threading import Lock
import uuid

Expand Down Expand Up @@ -461,11 +462,19 @@ def _delete_temporary_build_artifact_files(self):
ONLY call this method after _create_build_artifact() has completed. Otherwise we have lost the build results.
"""
build_result_dir = self._build_results_dir()
self._logger.info('Deleting build artifact files for {} in {}.', self._build_id, build_result_dir)
for path in os.listdir(build_result_dir):
# The build result tar-ball is also stored in this same directory, so we must not delete it.
if path == BuildArtifact.ARTIFACT_FILE_NAME:
continue
app.util.fs.async_delete(os.path.join(build_result_dir, path))
full_path = os.path.join(build_result_dir, path)
# Do NOT use app.util.fs.async_delete() here. That call will generate a temp directory for every
# atom, which can be in the thousands per build, and can lead to running up against the ulimit -Hn.
if os.path.isdir:
shutil.rmtree(full_path, ignore_errors=True)
else:
os.remove(full_path)
self._logger.info('Completed deleting artifact files for {}.', self._build_id)

def _build_results_dir(self):
return BuildArtifact.build_artifact_directory(self.build_id(), result_root=Configuration['results_directory'])
Expand Down

0 comments on commit ca2408a

Please sign in to comment.