Update Firestore Quickstart to use exec:exec#1523
Merged
BenWhitehead merged 1 commit intoJul 24, 2019
Merged
Conversation
kurtisvg
approved these changes
Jul 24, 2019
Contributor
|
@kurtisvg @nnegrey @averikitsch @dzlier-gcp These comments on exec:exec apply to more than just Firebase. We should be looking for this during reviews (and fix other samples) |
Contributor
Author
|
@lesv Is there anything else I need to do from your perspective before I can merge this? (I'll rebase it to be up to date with master). |
When exec-maven-plugin runs the `exec:java` goal, it executes the java program in the maven process (there currently isn't an option to fork instead of running it in process). As a side effect of this, shutdown hooks are not run when the program has completed running. In the case of our Quickstart that means the daemon thread for grpc and gax do not shutdown when the program has completed running, resulting in maven listing all the threads that are still running and showing a stacktrace. This change uses `exec:exec` and manually builds the java command to be run, thereby forcing the program to be ran in a forked process and able to use the normal shutdown process.
Contributor
|
@BenWhitehead No - I just wanted to call out exec:java to others. I see this being used a lot. I'm happy w/ what you are doing. |
Contributor
|
@lesv - I'll open an issue to reflect it in the sample format guidelines. |
BenWhitehead
added a commit
to googleapis/java-firestore
that referenced
this pull request
Aug 27, 2020
copied from original implementation in GoogleCloudPlatform/java-docs-samples#1523
This file contains hidden or 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
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.
When exec-maven-plugin runs the
exec:javagoal, it executes the javaprogram in the maven process (there currently isn't an option to fork
instead of running it in process). As a side effect of this, shutdown
hooks are not run when the program has completed running. In the case
of our Quickstart that means the daemon thread for grpc and gax do not
shutdown when the program has completed running, resulting in maven
listing all the threads that are still running and showing a stacktrace.
This change uses
exec:execand manually builds the java command to berun, thereby forcing the program to be ran in a forked process and able
to use the normal shutdown process.
Attached are a logs showing the following
exec:javaexec:javaexec:execexec_java.log
exec_java_thread_dump.log
exec_exec.log