I got a PR working to port

PyNailgunClient

. But killing Pantsd results in this:

The pantsd process was killed during the run.

If this was not intentionally done by you, Pants may have been killed by the operating system due to memory overconsumption (i.e. OOM-killed). You can set the global option `--pantsd-max-memory-usage` to reduce Pantsd's memory consumption by retaining less in its in-memory cache (run `./pants help-advanced global`). You can also disable pantsd with the global option `--pantsd` to avoid persisting memory across Pants runs, although you will miss out on additional caching.

If neither of those help, please consider filing a GitHub issue or reaching out on Slack so that we can investigate the possible memory overconsumption (<https://www.pantsbuild.org/docs/getting-help>).
Traceback (most recent call last):
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_loader.py", line 100, in run_default_entrypoint
    exit_code = runner.run(start_time)
  File "/Users/ericarellano/code/pants/src/python/pants/bin/pants_runner.py", line 86, in run
    return remote_runner.run()
  File "/Users/ericarellano/code/pants/src/python/pants/bin/remote_pants_runner.py", line 99, in run
    return self._connect_and_execute(pantsd_handle)
  File "/Users/ericarellano/code/pants/src/python/pants/bin/remote_pants_runner.py", line 131, in _connect_and_execute
    return PyNailgunClient(port, executor).execute(command, args, modified_env)
native_engine_pyo3.PantsdClientException: The pantsd process was killed during the run.

If this was not intentionally done by you, Pants may have been killed by the operating system due to memory overconsumption (i.e. OOM-killed). You can set the global option `--pantsd-max-memory-usage` to reduce Pantsd's memory consumption by retaining less in its in-memory cache (run `./pants help-advanced global`). You can also disable pantsd with the global option `--pantsd` to avoid persisting memory across Pants runs, although you will miss out on additional caching.

If neither of those help, please consider filing a GitHub issue or reaching out on Slack so that we can investigate the possible memory overconsumption (<https://www.pantsbuild.org/docs/getting-help>).

We only want that message to be printed once, not twice. This gets me wondering if that's probably because with Rust-CPython we have functions like

maybe_set_panic_handler

, and those prob don't apply to the PyO3 extension too?

This incremental migration might be less feasible than I hoped

For context, the PR https://github.com/pantsbuild/pants/pull/12181. Note the awkwardness with

PyExecutor

being defined in both cpython and PyO3. I do wonder if this migration needs to be done all at once

my guess re the “unclean exit” is just that you’re no longer raising the right exception type

should be a KeyboardInterrupt

is that just that the

str

or

repr

of the exception type changed…?

afaict, it doesn’t seem like a fundamental issue… just a small bug in the port

Ah red herring, my PR does nothing new with this error message.

main

behaves the same way. Yay!

I ported stdio to PyO3, but I'm trying to figure out why

./pants lint

now logs to the log file rather than console: https://github.com/pantsbuild/pants/pull/12184/commits/71b4220eaa2c57a85da5681ead7028e01bc0c160 I double checked my port and am pretty confident I did it one-for-one, except for using

Python.allow_threads()

in more places. Removing that doesn't make a difference Any ideas?

i doubt that Py03 does either

I'm working more on this before we leave for our trip in a little, and I'm souring on an incremental migration. After this stdio and Nailgun client PR, there's very little we can incrementally port, it's all deeply tied to Scheduler Then this mysterious issue with logging I also realize I want to be able to benchmark this migration and easily revert it / not land if it's a problem. That's hard to revert if we've incrementally ported -- @witty-crayon-22786 would you consider one big PR? I know that would be a beast to review

unless it turns out that it really is a python issue… i sortof doubt that it is

the thread-local access is not actually powered by python… it’s a rust-level thread local

the async / futures3 migration was incremental though, because we had the framework

yea, i do think that some of the changes will be big bang (everything reference the Scheduler). logging seems like a good choice to be independent though, since it is truly independent.

i can spend some time looking at the stdio issue if you’d like.

@hundreds-father-404: did you know about https://github.com/pantsbuild/pants/blob/3adc0e05912019b5c67959ce54422477cbf47eb9/src/rust/engine/logging/src/lib.rs#L30-L48 ?

if you get no output at all to the console, then you’d want to see whether it was appropriately set … ever

if you get some output to the console, and some that goes to the log, that would be the destination failing to be propagated from one thread/task to the other… but that’s unlikely, because it has nothing to do with python

if no output to console, you’d want to check that inside the call to stdio_thread_console_set you actually have one

so

stdio_thread_console_set

is not doing the right thing

it doesn’t look like any callers were updated

(unless i’m missing something)

does src/python/pants/init/logging.py need to be updated to import the new module?

yea, would confirm that inside the call to set the console that you actually end up with one.

but yea, i’ll try to take a look.

Thanks! Does it make sense to not merge Nailgun port until we solve this because it might impact whether incremental is possible or not? Maybe it's still fine. And even if console can't be figured out, we can still port most of stdio (I'm also thinking that matches_path_globs is a great incremental PR after all because it will set precedent for how to consume Python objects like PathGlobs)

i’m optimistic about this being worthwhile.

the commits are mucked up, but https://github.com/pantsbuild/pants/pull/12184 is now in its final state outside of this issue - I ported the panic handler this morning