[pantsd] Robustify client connection logic.

[kwlzn]

Fixes #5812

under full-on-assault stress testing via:

$ watch -n.1 'pkill -f "pantsd \[" pantsd-runner'

this will mostly behave like:

WARN] pantsd was unresponsive on port 55620, retrying (1/3)
WARN] pantsd was unresponsive on port 55620, retrying (2/3)
WARN] pantsd was unresponsive on port 55626, retrying (3/3)
WARN] caught client exception: Fallback(NailgunExecutionError(u'Problem executing command on nailgun server (address: 127.0.0.1:55630): TruncatedHeaderError(u"Failed to read nailgun chunk header (TruncatedRead(u\'Expected 5 bytes before socket shutdown, instead received 0\',)).",)',),), falling back to non-daemon mode

23:30:24 00:00 [main]
               (To run a reporting server: ./pants server)
23:30:38 00:14   [setup]
23:30:39 00:15     [parse]
...

mid-flight terminations (simulated via single-shot pkill calls) also result in a more descriptive error with traceback proxying:

23:40:51 00:04     [zinc]
23:40:51 00:04     [javac]
23:40:51 00:04     [cpp]
23:40:51 00:04     [errorprone]
23:40:51 00:04     [findbugs]CRITICAL]
CRITICAL] lost active connection to pantsd!
Exception caught: (<class 'pants.bin.remote_pants_runner.Terminated'>)
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 73, in <module>
    main()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 69, in main
    PantsLoader.run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 65, in run
    cls.load_and_execute(entrypoint)
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_loader.py", line 58, in load_and_execute
    entrypoint_main()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_exe.py", line 39, in main
    PantsRunner(exiter, start_time=start_time).run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/pants_runner.py", line 39, in run
    return RemotePantsRunner(self._exiter, self._args, self._env, bootstrap_options).run()
  File "/Users/kwilson/dev/pants/src/python/pants/bin/remote_pants_runner.py", line 162, in run
    self._run_pants_with_retry(port)
  File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_client.py", line 221, in execute
    return self._session.execute(cwd, main_class, *args, **environment)
  File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_client.py", line 94, in execute
    return self._process_session()
  File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_client.py", line 69, in _process_session
    for chunk_type, payload in self.iter_chunks(self._sock, return_bytes=True):
  File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_protocol.py", line 206, in iter_chunks
    chunk_type, payload = cls.read_chunk(sock, return_bytes)
  File "/Users/kwilson/dev/pants/src/python/pants/java/nailgun_protocol.py", line 182, in read_chunk
    raise cls.TruncatedHeaderError('Failed to read nailgun chunk header ({!r}).'.format(e))

Exception message: abruptly lost active connection to pantsd runner: NailgunError(u'Problem talking to nailgun server (address: 127.0.0.1:55707, remote_pid: -28972): TruncatedHeaderError(u"Failed to read nailgun chunk header (TruncatedRead(u\'Expected 5 bytes before socket shutdown, instead received 0\',)).",)',)

[wisechengyi]

Thanks @kwlzn. I am not too familiar with pantsd code, so left some questions.

[wisechengyi]

Just wondering if there is any particular reason to use attempt + (attempt - 1)?

[kwlzn]

just trying to keep the retry overhead to <10 seconds and this staggers nicely as 1, 3, 5 seconds for 3 retries. the backoff curve here needn't be as sharp as e.g. a distributed system where things like thundering herd are a thing.

[wisechengyi]

Is it possible there are dangling pantd processes that need to be killed before restarting one?

[kwlzn]

PantsDaemon.launch() already calls ProcessManager.terminate() to handle that case here: https://github.com/pantsbuild/pants/blob/master/src/python/pants/pantsd/pants_daemon.py#L393

[wisechengyi]

Ah ok. Thank you!

[illicitonion]

Looks good!

Would it be possible to test this with a test which runs a kill -9 on the pantsd pid or something, or is the timing too sensitive to easily simulate this in a test?

[stuhood]

Thanks, looks good!

[kwlzn]

@illicitonion "is the timing too sensitive to easily simulate this in a test?" -> exactly. there's also still a slight possibility of an occasional failure under the right conditions (the retry logic can get far enough into a connection that it won't be retryable, so it may get a Terminated on subsequent retry under the constant pkill conditions) - so I think at best an integration test would be pretty flaky/unreliable, so I was hesitant to introduce that to CI.