Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Firestore Quickstart to use exec:exec #1523

Merged
merged 1 commit into from Jul 24, 2019
Merged

Update Firestore Quickstart to use exec:exec #1523

merged 1 commit into from Jul 24, 2019

Conversation

BenWhitehead
Copy link
Contributor

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.

Attached are a logs showing the following

  • Errors printed by maven when using exec:java
  • A thread dump of the maven process when using exec:java
  • A log without errors after changing to exec:exec

exec_java.log
exec_java_thread_dump.log
exec_exec.log

@BenWhitehead BenWhitehead requested review from kurtisvg and a team July 24, 2019 16:00
@BenWhitehead BenWhitehead self-assigned this Jul 24, 2019
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jul 24, 2019
firestore/pom.xml Show resolved Hide resolved
@lesv
Copy link
Contributor

lesv commented Jul 24, 2019

@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)

@BenWhitehead
Copy link
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.
@lesv
Copy link
Contributor

lesv commented Jul 24, 2019

@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.

@kurtisvg
Copy link
Contributor

@lesv - I'll open an issue to reflect it in the sample format guidelines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants