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

Kill pants.bootstrap and move its functionality to pants. #12

Closed
wants to merge 3 commits into from

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Apr 2, 2014

There is no reason to ever run pants in the pants repo
from an always-old released versio, so kill pants in its
prior form and move the pants.bootstrap functionality there.

Fixup pants to use requirement.txt files to control 3rdparty
deps. This allows the BUILDs and the bootstrap to say in
sync with no effort.

There is no reason to ever run pants in the pants repo
from an always-old released versio, so kill pants in its
prior form and move the pants.bootstrap functionality there.

Fixup pants to use requirement.txt files to control 3rdparty
deps.  This allows the BUILDs and the bootstrap to say in
sync with no effort.
@jsirois
Copy link
Contributor Author

jsirois commented Apr 2, 2014

I'll add tests for python_requirements after you ogle the approach. This is fully working.

John Sirois added 2 commits April 2, 2014 16:57
…tstrap

Conflicts:
	3rdparty/python/BUILD
	3rdparty/python/twitter/common/BUILD
	pants.bootstrap
	src/python/pants/BUILD
	src/python/pants/BUILD.transitional
	src/python/pants/base/BUILD
	src/python/pants/bin/BUILD
	src/python/pants/cache/BUILD
	src/python/pants/commands/BUILD
	src/python/pants/engine/BUILD
	src/python/pants/fs/BUILD
	src/python/pants/goal/BUILD
	src/python/pants/ivy/BUILD
	src/python/pants/java/BUILD
	src/python/pants/net/BUILD
	src/python/pants/python/BUILD
	src/python/pants/reporting/BUILD
	src/python/pants/scm/BUILD
	src/python/pants/targets/BUILD
	src/python/pants/targets/python_target.py
	src/python/pants/tasks/BUILD
	src/python/pants/tasks/jvm_compile/BUILD
	src/python/pants/tasks/python/BUILD
Revert src/python/pants/python/interpreter_cache.py change
in favor of specifying a proper bytestring in py.py.

Kill unused 3rdparty/python/twitter/common/BUILD.

Fixup comments in 3rdparty/
# Copyright 2014 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_requirements()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why have these in a separate file? They are just 3rdparty deps now. I had already moved them into 3rdparty/python:BUILD.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They have a different --find-links (when pulling from pantsbuild/cheeseshop) so this is a technical hard requirement.

@benjyw
Copy link
Contributor

benjyw commented Apr 2, 2014

Approach seems fine, other than: why have the commons deps be in a separate
BUILD file? I had already moved them into 3rdparty/python/BUILD.

On Wed, Apr 2, 2014 at 3:45 PM, John Sirois [email protected]:

I'll add tests for python_requirements after you ogle the approach. This
is fully working.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12#issuecomment-39393529
.

@jsirois
Copy link
Contributor Author

jsirois commented Apr 2, 2014

Since the commons deps can be pulled from pantsbuild/cheeseshop - this is a hard requirement. The difficulty is that with just --find-links (and the pypi index in-play) - the pypi latest commons sdists will be picked. So the 3rdparty/python/twitter/commons/requirments.txt has to say --no-index at which point it can't resolve the stuff in 3rdparty/python/requirements.txt which are all on pypi.

@benjyw
Copy link
Contributor

benjyw commented Apr 2, 2014

OK, but long run we want pypi commons sdists (at a fixed version) to be
picked, no? I'm averse to special-casing twitter/commons.

On Wed, Apr 2, 2014 at 4:18 PM, John Sirois [email protected]:

Since the commons deps can be pulled from pantsbuild/cheeseshop - this is
a hard requirement. The difficulty is that with just --find-links (and the
pypi index in-play) - the pypi latest commons sdists will be picked. So the
3rdparty/python/twitter/commons/requirments.txt has to say --no-index at
which point it can't resolve the stuff in 3rdparty/python/requirements.txt
which are all on pypi.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12#issuecomment-39396103
.

@jsirois
Copy link
Contributor Author

jsirois commented Apr 2, 2014

Agreed - note the comments in the requirements.txt file. That said, take it or leave it - as things stand it must be this way, or at least commons must be in their own requirements file in 3rdparty/python, so maybe 3rdparty/python/commons-requirements.txt.

Also note, we have inside twitter a standard of this sort of layout on its own merits, ie: 3rdaprty/jvm/com/foursquare/BUILD for foursquare artifacts.

@benjyw
Copy link
Contributor

benjyw commented Apr 2, 2014

Yeah, it's fine for now. LGTM. Ship it. Whatever the code word is...

On Wed, Apr 2, 2014 at 4:28 PM, John Sirois [email protected]:

Agreed - note the comments in the requirements.txt file. That said, take
it or leave it - as things stand it must be this way, or at least commons
must be in their own requirements file in 3rdparty/python, so maybe
3rdparty/python/commons-requirements.txt.

Also note, we have inside twitter a standard of this sort of layout on its
own merits, ie: 3rdaprty/jvm/com/foursquare/BUILD for foursquare artifacts.


Reply to this email directly or view it on GitHubhttps://github.com//pull/12#issuecomment-39396784
.

@jsirois
Copy link
Contributor Author

jsirois commented Apr 2, 2014

Thanks - merged

@jsirois jsirois closed this Apr 2, 2014
@jsirois jsirois deleted the jsirois/bootstrap branch April 2, 2014 23:42
kwlzn added a commit that referenced this pull request Mar 31, 2017
…n test. (#4407)

### Problem

Currently, on Linux the first thin client call to the daemon can deadlock just after the pantsd->fork->pantsd-runner workflow. Connecting to the process with `gdb` reveals a deadlock in the following stack in the `post_fork` `drop` of the `CpuPool`:

```
#0  0x00007f63f04c31bd in __lll_lock_wait () from /lib64/libpthread.so.0
No symbol table info available.
#1  0x00007f63f04c0ded in pthread_cond_signal@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
No symbol table info available.
#2  0x00007f63d3cfa438 in notify_one ()
    at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys/unix/condvar.rs:52
No locals.
#3  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys_common/condvar.rs:39
No locals.
#4  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sync/condvar.rs:208
No locals.
#5  std::thread::{{impl}}::unpark () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/thread/mod.rs:633
No locals.
#6  0x00007f63d3c583d1 in crossbeam::sync::ms_queue::{{impl}}::push<futures_cpupool::Message> (self=<optimized out>, t=...)
    at /home/kwilson/.cache/pants/rust-toolchain/registry/src/github.com-1ecc6299db9ec823/crossbeam-0.2.10/src/sync/ms_queue.rs:178
        guard = <optimized out>
        self = <optimized out>
#7  0x00007f63d3c588ed in futures_cpupool::{{impl}}::drop (self=<optimized out>)
    at /home/kwilson/.cache/pants/rust-toolchain/git/checkouts/futures-rs-a4f11d094efefb0a/f7e6bc8/futures-cpupool/src/lib.rs:236
        self = 0x37547a0
#8  0x00007f63d3be871c in engine::fs::{{impl}}::post_fork (self=0x3754778) at /home/kwilson/dev/pants/src/rust/engine/src/fs.rs:355
        self = 0x3754778
#9  0x00007f63d3be10e4 in engine::context::{{impl}}::post_fork (self=0x37545b0)
    at /home/kwilson/dev/pants/src/rust/engine/src/context.rs:93
        self = 0x37545b0
#10 0x00007f63d3c0de5a in {{closure}} (scheduler=<optimized out>) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:275
        scheduler = 0x3740580
#11 with_scheduler<closure,()> (scheduler_ptr=<optimized out>, f=...) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:584
        scheduler = 0x3740580
        scheduler_ptr = 0x3740580
#12 engine::scheduler_post_fork (scheduler_ptr=0x3740580) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:274
        scheduler_ptr = 0x3740580
#13 0x00007f63d3c1be8c in _cffi_f_scheduler_post_fork (self=<optimized out>, arg0=0x35798f0) at src/cffi/native_engine.c:2234
        _save = 0x34a65a0
        x0 = 0x3740580
        datasize = <optimized out>
#14 0x00007f63f07b5a62 in PyEval_EvalFrameEx () from /lib64/libpython2.7.so.1.0
```

This presents as a hang in the thin client, because the pailgun socket is left open in the pantsd-runner.

### Solution

Add pre-fork hooks and tear down the `CpuPool` instances prior to forking and rebuilding them.

### Result

Can no longer reproduce the hang.
lenucksi pushed a commit to lenucksi/pants that referenced this pull request Apr 25, 2017
…n test. (pantsbuild#4407)

### Problem

Currently, on Linux the first thin client call to the daemon can deadlock just after the pantsd->fork->pantsd-runner workflow. Connecting to the process with `gdb` reveals a deadlock in the following stack in the `post_fork` `drop` of the `CpuPool`:

```
#0  0x00007f63f04c31bd in __lll_lock_wait () from /lib64/libpthread.so.0
No symbol table info available.
pantsbuild#1  0x00007f63f04c0ded in pthread_cond_signal@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
No symbol table info available.
pantsbuild#2  0x00007f63d3cfa438 in notify_one ()
    at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys/unix/condvar.rs:52
No locals.
pantsbuild#3  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sys_common/condvar.rs:39
No locals.
pantsbuild#4  notify_one () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/sync/condvar.rs:208
No locals.
pantsbuild#5  std::thread::{{impl}}::unpark () at /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libstd/thread/mod.rs:633
No locals.
pantsbuild#6  0x00007f63d3c583d1 in crossbeam::sync::ms_queue::{{impl}}::push<futures_cpupool::Message> (self=<optimized out>, t=...)
    at /home/kwilson/.cache/pants/rust-toolchain/registry/src/github.com-1ecc6299db9ec823/crossbeam-0.2.10/src/sync/ms_queue.rs:178
        guard = <optimized out>
        self = <optimized out>
pantsbuild#7  0x00007f63d3c588ed in futures_cpupool::{{impl}}::drop (self=<optimized out>)
    at /home/kwilson/.cache/pants/rust-toolchain/git/checkouts/futures-rs-a4f11d094efefb0a/f7e6bc8/futures-cpupool/src/lib.rs:236
        self = 0x37547a0
pantsbuild#8  0x00007f63d3be871c in engine::fs::{{impl}}::post_fork (self=0x3754778) at /home/kwilson/dev/pants/src/rust/engine/src/fs.rs:355
        self = 0x3754778
pantsbuild#9  0x00007f63d3be10e4 in engine::context::{{impl}}::post_fork (self=0x37545b0)
    at /home/kwilson/dev/pants/src/rust/engine/src/context.rs:93
        self = 0x37545b0
pantsbuild#10 0x00007f63d3c0de5a in {{closure}} (scheduler=<optimized out>) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:275
        scheduler = 0x3740580
pantsbuild#11 with_scheduler<closure,()> (scheduler_ptr=<optimized out>, f=...) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:584
        scheduler = 0x3740580
        scheduler_ptr = 0x3740580
pantsbuild#12 engine::scheduler_post_fork (scheduler_ptr=0x3740580) at /home/kwilson/dev/pants/src/rust/engine/src/lib.rs:274
        scheduler_ptr = 0x3740580
pantsbuild#13 0x00007f63d3c1be8c in _cffi_f_scheduler_post_fork (self=<optimized out>, arg0=0x35798f0) at src/cffi/native_engine.c:2234
        _save = 0x34a65a0
        x0 = 0x3740580
        datasize = <optimized out>
pantsbuild#14 0x00007f63f07b5a62 in PyEval_EvalFrameEx () from /lib64/libpython2.7.so.1.0
```

This presents as a hang in the thin client, because the pailgun socket is left open in the pantsd-runner.

### Solution

Add pre-fork hooks and tear down the `CpuPool` instances prior to forking and rebuilding them.

### Result

Can no longer reproduce the hang.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants