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

[bug] concurrency errors in conan export #11480

Closed
SSE4 opened this issue Jun 18, 2022 · 17 comments
Closed

[bug] concurrency errors in conan export #11480

SSE4 opened this issue Jun 18, 2022 · 17 comments

Comments

@SSE4
Copy link
Contributor

SSE4 commented Jun 18, 2022

Environment Details (include every applicable attribute)

  • Operating System+version: macOS 12.3
  • Compiler+version: Apple clang version 13.1.6 (clang-1316.0.21.2.5)
  • Conan version: Conan version 2.0.0-alpha7
  • Python version: Python 3.7.1

Steps to reproduce (Include if Applicable)

launch a simple script:

import subprocess
import multiprocessing

text = """from conan import ConanFile
class MyConanFile(ConanFile):
    pass
"""

with open("conanfile.py", "w") as f:
    f.write(text)

def run():
    while True:
        try:
            subprocess.check_call(["conan", "export", ".", "--name", "mylib", "--version", "1.0"])
        except:
            pass

jobs = []
for i in range(0, 2):
    process = multiprocessing.Process(target=run)
    jobs.append(process)
for j in jobs:
    j.start()
for j in jobs:
    j.join()

Logs (Executed commands with output) (Include/Attach if Applicable)

observe various random errors:

Traceback (most recent call last):
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/shutil.py", line 557, in move
    os.rename(src, real_dst)
OSError: [Errno 66] Directory not empty: '/Users/sse4/.conan2/p/tmp/e9336c83458230ec' -> '/Users/sse4/.conan2/p/0878a4b138b80085'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conans/cli/cli.py", line 163, in run
    command.run(self._conan_api, self._commands[command_argument].parser, args[0][1:])
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conans/cli/command.py", line 157, in run
    info = self._method(conan_api, parser, *args)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conans/cli/commands/export.py", line 39, in export
    lockfile=lockfile)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conan/api/subapi/__init__.py", line 21, in wrapper
    return f(subapi, *args, **kwargs)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conan/api/subapi/export.py", line 21, in export
    return cmd_export(app, path, name, version, user, channel, graph_lock=lockfile)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conans/client/cmd/export.py", line 62, in cmd_export
    cache.assign_rrev(recipe_layout)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conans/client/cache/cache.py", line 60, in assign_rrev
    return self._data_cache.assign_rrev(layout)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/site-packages/conan/cache/cache.py", line 224, in assign_rrev
    shutil.move(self._full_path(layout.base_folder), full_path)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/shutil.py", line 568, in move
    symlinks=True)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/shutil.py", line 315, in copytree
    os.makedirs(dst)
  File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/os.py", line 221, in makedirs
    mkdir(name, mode)
FileExistsError: [Errno 17] File exists: '/Users/sse4/.conan2/p/0878a4b138b80085'
 Traceback (most recent call last):
   File "/Users/sse4/.pyenv/versions/3.7.1/lib/python3.7/shutil.py", line 557, in move
    os.rename(src, real_dst)
OSError: [Errno 66] Directory not empty: '/Users/sse4/.conan2/p/tmp/b366d60a7e3773f7' -> '/Users/sse4/.conan2/p/0878a4b138b80085'
ERROR: Couldn't remove folder, might be busy or open: /Users/sse4/.conan2/p/0878a4b138b80085
@memsharded
Copy link
Member

This is not a bug, this is per-design.
The Conan cache in 2.0 is not concurrent, you need to do the exports sequentially.

We will eventually work on concurrency after 2.0, but at the moment this is not a bug.

@SSE4
Copy link
Contributor Author

SSE4 commented Jun 19, 2022

I struggle to find the information, maybe @czoido or @jgsogo can clarify. I only found some earlier PRs about new 2.0 cache (#8510, #8796, #9164), and these PRs mentioned concurrency support.
anyway, it worked correctly in 1.x, so it's a regression in 2.0.

@memsharded
Copy link
Member

Conan 1.X never promised nor documented to be concurrency-safe, so in that sense it cannot be a regression.

Here you can find the summary for 2.0 that indicates the cache is not concurrent: conan-io/docs#2593

@lo1ol
Copy link

lo1ol commented Mar 6, 2023

I faced with the same problem on "conan install".

Is it not a bug too?

@lo1ol
Copy link

lo1ol commented Mar 6, 2023

12:53:49.043 ======== Computing dependency graph ========
12:53:49.043 cprocsp/5.0.11732@rutoken/experimental: Not found in local cache, looking in remotes...
12:53:49.043 cprocsp/5.0.11732@rutoken/experimental: Checking remote: conancenter
12:53:49.043 cprocsp/5.0.11732@rutoken/experimental: Checking remote: rutoken
12:53:49.043 ERROR: [Errno 17] File exists: '/home/jenkins/.conan2/p/cprocb60939207fe33/e'

@memsharded
Copy link
Member

Yes, at the moment cache is not concurrent: https://docs.conan.io/2/knowledge/guidelines.html

There could be some tricks that would allow some concurrency, for example, if the conan install is done in parallel to install multiple configurations it is likely that:

  • conan graph info to install the "common" part of the graph, that is, the recipes, first, no parallel.
  • for each config do conan install concurrently. As the binary packages have different identifiers, it is less likely that they will be creating race conditions in the cache

@lo1ol
Copy link

lo1ol commented Mar 7, 2023

It seems better to use self-written wrapper for me. Something like that:

#!/bin/env bash

LOCKFILE=~/rutoken_conan.lock

(
flock 9 || exit 1
conan $@
) 9>${LOCKFILE}

Our CI may build different project with the same profile at the same time

@memsharded
Copy link
Member

Sure, with the wrapper you probably won't hit any issues at all, because you are doing strictly sequential usage of the cache, isn't it?
The solution that I was outlining above might be able to achieve some concurrency, because the slowest part is installing the binaries, for different configurations, but it is true that if it is a more general concurrency issue, your approach would be better.

@lo1ol
Copy link

lo1ol commented Mar 7, 2023

The problem is that machines at our CI may build several projects with the same profile at the same time. And it's hardly to avoid this.

So, to avoid concurrent I have to disable cache at all (make it local) or use global lock file.

Btw, I'm working on another solution, based on custom commands:

from conan.cli.command import conan_command
from conan.cli.commands.install import install, json_install
from conans.util.locks import SimpleLock

from os import path

@conan_command(group="Custom commands", formatters={"json": json_install})
def rtinstall(conan_api, parser, *args):
    """
    Concurrency-safe wrapper over install cmd.
    """
    
    with SimpleLock(path.join(conan_api.cache_folder, "global.lock")):
        return install.run(conan_api, parser, *args)

@memsharded
Copy link
Member

Thanks for the feedback, seems a reasonable approach

I like the idea of a global lock over a cache, that would be quite safe, and avoid difficult to debug race conditions.
But I am also afraid that some users will inevitably complain, because they have their own usage patterns, and they know that building 2 different packages in parallel is safe for them...

@lo1ol
Copy link

lo1ol commented Mar 7, 2023

Got you. Each case of usage should have own most efficient approach)

Btw.
Is it safe to use SimpleLock from conans.util.locks (and other imported conan objects)? Is it may disappear in future releases?

@memsharded
Copy link
Member

Is it safe to use SimpleLock from conans.util.locks (and other imported conan objects)? Is it may disappear in future releases?

It is very possible that the moment we start to address the cache concurrency, these objects might suffer modifications. Not be removed, but likely to have breaking changes, even renames. So in the general case not recommended for usage. In any case that is a relatively thin layer over fasteners you might duplicate it, or take the risk, it is not that it will be terribly complicated to fix when it breaks.

@lo1ol
Copy link

lo1ol commented Mar 7, 2023

I noticed that --format doesn't work with this command. So I replace install.run to install._method:

from conan.cli.command import conan_command
from conan.cli.commands.install import install, json_install
from conans.util.locks import SimpleLock

from os import path

@conan_command(group="Custom commands", formatters={"json": json_install})
def rtinstall(conan_api, parser, *args):
    """
    Concurrency-safe wrapper over install cmd.
    """
    
    with SimpleLock(path.join(conan_api.cache_folder, "global.lock")):
        return install._method(conan_api, parser, *args)

May you suggest another approach how to rightly wrap conan commands?

@memsharded
Copy link
Member

Oh, yes, the commands are not designed to be directly called from other commands, that is what could be causing that --format doesn't work. That private _method would be another possible point of breaking, but if it works for you in the meantime and you are aware of the risk and ready to fix it if a future 2.X release breaks it, then, it might be worth, it is just a few lines, not hundreds of them to fix.

@lo1ol
Copy link

lo1ol commented Mar 6, 2024

@memsharded Hello! This feature still not implemented?

@memsharded
Copy link
Member

Oh, yes, the commands are not designed to be directly called from other commands,

This has already improved in #15630, it will be in next 2.2 release. It allows to call other commands from any other command via a new CliAPI. Please keep an eye on it, and let us know if there are any issue with it.

The cache concurrency, still not there. It is a challenging feature that will require time, and we still have many other higher priority issues.

@memsharded
Copy link
Member

Closing in favor of #15840 where the Conan 2 cache concurrency future progress will be reported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants