syahra712 commented on code in PR #146:
URL: https://github.com/apache/solr-orbit/pull/146#discussion_r4433075115
##########
solrorbit/builder/solr_provisioner.py:
##########
@@ -436,11 +440,27 @@ def _cluster_config_env_flags(self) -> list:
flags += ["-e", f"SOLR_OPTS={solr_opts}"]
return flags
- def stop(self) -> None:
- """Stop and remove the Solr container."""
- cmd = ["docker", "rm", "-f", self.container_name]
+ def stop(self, log_dest: str = None) -> None:
+ """
+ Stop the Solr container, copy its log directory to *log_dest* (if
given), and remove it.
+
+ Stopping first lets the JVM flush files written on exit, such as the
JFR recording.
+ """
logger.info("Stopping Solr container '%s'", self.container_name)
- result = subprocess.run(cmd, capture_output=True, text=True)
+ result = subprocess.run(["docker", "stop", self.container_name],
capture_output=True, text=True)
+ if result.returncode != 0:
+ logger.warning("docker stop returned non-zero: %s", result.stderr
or result.stdout)
+ elif log_dest:
+ os.makedirs(log_dest, exist_ok=True)
+ # The trailing "/." copies the directory's contents rather than
the directory itself.
+ source = f"{self.container_name}:{self.CONTAINER_LOG_DIR}/."
+ result = subprocess.run(["docker", "cp", source, log_dest],
capture_output=True, text=True)
+ if result.returncode != 0:
+ logger.warning("Could not copy Solr logs from the container:
%s", result.stderr or result.stdout)
+ else:
+ logger.info("Copied Solr logs from the container to %s",
log_dest)
+
+ result = subprocess.run(["docker", "rm", "-f", self.container_name],
capture_output=True, text=True)
Review Comment:
Good catch, confirmed on `solr:9`: `docker rm -f` leaves the `/var/solr`
volume behind and `-v` removes it. Both `stop()` and the stale-container
removal in `start()` now use `rm -f -v` (ffbb81f).
##########
solrorbit/builder/solr_provisioner.py:
##########
@@ -436,11 +440,27 @@ def _cluster_config_env_flags(self) -> list:
flags += ["-e", f"SOLR_OPTS={solr_opts}"]
return flags
- def stop(self) -> None:
- """Stop and remove the Solr container."""
- cmd = ["docker", "rm", "-f", self.container_name]
+ def stop(self, log_dest: str = None) -> None:
+ """
+ Stop the Solr container, copy its log directory to *log_dest* (if
given), and remove it.
+
+ Stopping first lets the JVM flush files written on exit, such as the
JFR recording.
+ """
logger.info("Stopping Solr container '%s'", self.container_name)
- result = subprocess.run(cmd, capture_output=True, text=True)
+ result = subprocess.run(["docker", "stop", self.container_name],
capture_output=True, text=True)
+ if result.returncode != 0:
+ logger.warning("docker stop returned non-zero: %s", result.stderr
or result.stdout)
+ elif log_dest:
+ os.makedirs(log_dest, exist_ok=True)
Review Comment:
Agreed. `docker rm` is now in a `finally`, so it runs even if creating
`log_dest` raises. Added a test for that (ffbb81f).
##########
solrorbit/test_run_orchestrator.py:
##########
@@ -607,14 +657,17 @@ def solr_docker(cfg):
cc_instance = _load_cluster_config(cfg)
solr_modules = cfg.opts("solr", "modules", mandatory=False,
default_value="")
- launcher = SolrDockerLauncher(port=port, cluster_config=cc_instance,
solr_modules=solr_modules)
+ jvm_devices = _create_jvm_telemetry_devices(cfg,
container_log_root=SolrDockerLauncher.CONTAINER_LOG_DIR)
+ launcher = SolrDockerLauncher(port=port, cluster_config=cc_instance,
solr_modules=solr_modules,
+ telemetry_devices=jvm_devices)
Review Comment:
Went with your suggestion: `-XX:+PrintAssembly` is now opt-in via
`--telemetry-params jit-print-assembly:true` (from #143, merged in here), so
`--telemetry jit` starts within the 60s timeout by default. The docs explain
the cost.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
