[Notes] [Git][BuildStream/buildstream][tristan/fix-cache-size-race] 5 co

Tristan Van Berkom pushed to branch tristan/fix-cache-size-race at BuildStream / buildstream

Commits:

3bf895d2

by Jonathan Maw at 2018-10-03T11:48:48Z

tests: Add regression test for mirroring with project.refs

e4969807

by Jonathan Maw at 2018-10-03T12:48:07Z

Merge branch 'jonathan/source-mirror-project-refs' into 'master'

tests: Add regression test for mirroring with project.refs

See merge request BuildStream/buildstream!823

5f408fc3

by Tristan Van Berkom at 2018-10-03T13:06:16Z

utils.py: Give save_file_atomic() a tempdir argument

Allow callers to decide where the temporary file will be created.

f66604ec

by Tristan Van Berkom at 2018-10-03T13:06:16Z

_artifactcache/cascache.py: Don't create temporary files in the CAS storage

Use the designated tempdir when creating refs, we expect that temporary
files are not created in the storage directory ever, they should be
only ever created in the designated temporary directory.

This fixes race conditions where utils._get_dir_size() throws an
unhandled exception when attempting to stat the file which inadvertantly
disappears.

5c6183a8

by Tristan Van Berkom at 2018-10-03T13:06:16Z

utils.py: Document _get_dir_size() expectations.

This function assumes that files do not disappear while
walking the given directory.

Changes:

buildstream/_artifactcache/cascache.py

@@ -506,7 +506,7 @@ class CASCache(ArtifactCache):
      def set_ref(self, ref, tree):
          refpath = self._refpath(ref)
          os.makedirs(os.path.dirname(refpath), exist_ok=True)
 -        with utils.save_file_atomic(refpath, 'wb') as f:
 +        with utils.save_file_atomic(refpath, 'wb', tempdir=self.tmpdir) as f:
              f.write(tree.SerializeToString())
      # resolve_ref():

buildstream/utils.py

@@ -502,7 +502,7 @@ def get_bst_version():
  @contextmanager
  def save_file_atomic(filename, mode='w', *, buffering=-1, encoding=None,
 -                     errors=None, newline=None, closefd=True, opener=None):
 +                     errors=None, newline=None, closefd=True, opener=None, tempdir=None):
      """Save a file with a temporary name and rename it into place when ready.
      This is a context manager which is meant for saving data to files.
@@ -529,8 +529,9 @@ def save_file_atomic(filename, mode='w', *, buffering=-1, encoding=None,
      # https://bugs.python.org/issue8604
      assert os.path.isabs(filename), "The utils.save_file_atomic() parameter ``filename`` must be an absolute path"
 -    dirname = os.path.dirname(filename)
 -    fd, tempname = tempfile.mkstemp(dir=dirname)
 +    if tempdir is None:
 +        tempdir = os.path.dirname(filename)
 +    fd, tempname = tempfile.mkstemp(dir=tempdir)
      os.close(fd)
      f = open(tempname, mode=mode, buffering=buffering, encoding=encoding,
@@ -562,6 +563,9 @@ def save_file_atomic(filename, mode='w', *, buffering=-1, encoding=None,
+ #
  # Get the disk usage of a given directory in bytes.
+ #
 +# This function assumes that files do not inadvertantly
 +# disappear while this function is running.
 +#
  # Arguments:
  #     (str) The path whose size to check.
+ #

tests/frontend/mirror.py

@@ -139,6 +139,82 @@ def test_mirror_fetch(cli, tmpdir, datafiles, kind):
      result.assert_success()
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.parametrize("ref_storage", [("inline"), ("project.refs")])
 +@pytest.mark.parametrize("mirror", [("no-mirror"), ("mirror"), ("unrelated-mirror")])
 +def test_mirror_fetch_ref_storage(cli, tmpdir, datafiles, ref_storage, mirror):
 +    bin_files_path = os.path.join(str(datafiles), 'files', 'bin-files', 'usr')
 +    dev_files_path = os.path.join(str(datafiles), 'files', 'dev-files', 'usr')
 +    upstream_repodir = os.path.join(str(tmpdir), 'upstream')
 +    mirror_repodir = os.path.join(str(tmpdir), 'mirror')
 +    project_dir = os.path.join(str(tmpdir), 'project')
 +    os.makedirs(project_dir)
 +    element_dir = os.path.join(project_dir, 'elements')
++
 +    # Create repo objects of the upstream and mirror
 +    upstream_repo = create_repo('tar', upstream_repodir)
 +    upstream_ref = upstream_repo.create(bin_files_path)
 +    mirror_repo = upstream_repo.copy(mirror_repodir)
 +    mirror_ref = upstream_ref
 +    upstream_ref = upstream_repo.create(dev_files_path)
++
 +    element = {
 +        'kind': 'import',
 +        'sources': [
 +            upstream_repo.source_config(ref=upstream_ref if ref_storage == 'inline' else None)
 +        ]
 +    }
 +    element_name = 'test.bst'
 +    element_path = os.path.join(element_dir, element_name)
 +    full_repo = element['sources'][0]['url']
 +    upstream_map, repo_name = os.path.split(full_repo)
 +    alias = 'foo'
 +    aliased_repo = alias + ':' + repo_name
 +    element['sources'][0]['url'] = aliased_repo
 +    full_mirror = mirror_repo.source_config()['url']
 +    mirror_map, _ = os.path.split(full_mirror)
 +    os.makedirs(element_dir)
 +    _yaml.dump(element, element_path)
++
 +    if ref_storage == 'project.refs':
 +        # Manually set project.refs to avoid caching the repo prematurely
 +        project_refs = {'projects': {
 +            'test': {
 +                element_name: [
 +                    {'ref': upstream_ref}
 +                ]
 +            }
 +        }}
 +        project_refs_path = os.path.join(project_dir, 'project.refs')
 +        _yaml.dump(project_refs, project_refs_path)
++
 +    project = {
 +        'name': 'test',
 +        'element-path': 'elements',
 +        'aliases': {
 +            alias: upstream_map + "/"
 +        },
 +        'ref-storage': ref_storage
 +    }
 +    if mirror != 'no-mirror':
 +        mirror_data = [{
 +            'name': 'middle-earth',
 +            'aliases': {alias: [mirror_map + '/']}
 +        }]
 +        if mirror == 'unrelated-mirror':
 +            mirror_data.insert(0, {
 +                'name': 'narnia',
 +                'aliases': {'frob': ['http://www.example.com/repo']}
 +            })
 +        project['mirrors'] = mirror_data
++
 +    project_file = os.path.join(project_dir, 'project.conf')
 +    _yaml.dump(project, project_file)
++
 +    result = cli.run(project=project_dir, args=['fetch', element_name])
 +    result.assert_success()
++
++
  @pytest.mark.datafiles(DATA_DIR)
  @pytest.mark.parametrize("kind", [(kind) for kind in ALL_REPO_KINDS])
  def test_mirror_fetch_upstream_absent(cli, tmpdir, datafiles, kind):

[Notes] [Git][BuildStream/buildstream][tristan/fix-cache-size-race] 5 commits: tests: Add regression test for mirroring with project.refs

Tristan Van Berkom pushed to branch tristan/fix-cache-size-race at BuildStream / buildstream

Commits:

3 changed files:

Changes: