Simplify SubprocessReportGenerator #359
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andrewazores
force-pushed
the
subprocess-report-simplify
branch
from
154a3ce to
b0f405a
Compare
Parent process produces recording file in-process before forking report generation subprocess. Subprocess is simplified to always expect to be provided a path to a recording file, rather than either a path to a file or a targetId from which to retrieve a recording.
andrewazores
force-pushed
the
subprocess-report-simplify
branch
from
b0f405a to
d4da3aa
Compare
vic-ma
approved these changes
Dec 17, 2020
andrewazores
added a commit
to andrewazores/cryostat
that referenced
this pull request
Dec 17, 2020
Parent process produces recording file in-process before forking report generation subprocess. Subprocess is simplified to always expect to be provided a path to a recording file, rather than either a path to a file or a targetId from which to retrieve a recording.
tthvo
pushed a commit
to tthvo/cryostat-legacy
that referenced
this pull request
Dec 9, 2024
…atio#359) Bumps commons-io:commons-io from 2.13.0 to 2.16.1. --- updated-dependencies: - dependency-name: commons-io:commons-io dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, subprocessed report generation was triggered by either the
ActiveRecordingReportCacheorArchivedRecordingReportCache. TheActiveRecordingReportCachewould provide a description of the target (connection URL and any credentials) and the recording name, and theArchivedRecordingReportCachewould provide a local filesystem URI to a recording file already on-disk. The subprocess would then determine which it was provided at run time and either directly process the archived recording into a report, or set up its own connection toolkit and connection manager, then connect to the target application directly, copy the recording stream to a file, and then proceed to process it and generate a report. Then the report is written to a file and the subprocess shuts down.This was needlessly complex for the subprocess and required a bit of a hack to pass an already-saved archived recording file path to the subprocess as it it were a targetId, and then the subprocess has to check for this special case.
This PR refactors the flow so that the parent process is responsible for providing the subprocess with a path to a recording on disk in all cases. If the caller is the
ArchivedrRecordingReportCachethen this is already taken care of and the path is simply passed in. If the caller is theActiveRecordingReportCachethen it calls through to a utility method which uses the parent process' existing connection manager to copy the recording data into a local file, and then proceeds to pass the path to that file to the subprocess. The subprocess then always expects to be provided with consistent arguments - the path to the recording on disk, and the path where it should write the report.This also removes the need for the subprocess to handle establishing a JMX connection, or handling SSL/TLS certs for this connection, or handling JMX authorization credentials, etc., and should also reduce overall request->response latency for report requests since the parent process' target connection can be reused for retrieving the report (if necessary), a cached DNS entry for the target can be used if any lookup is needed, etc.