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

payara-3696-acc-executor-service #3936

Merged

Conversation

Projects
None yet
3 participants
@arjantijms
Copy link
Member

commented May 7, 2019

Payara-3696 Don't init executor service when in ACC

Hint for reviewers; review per individual commit. The first commit is adding a debug option to the ACC start, since it's otherwise quite difficult to debug the agent code for the ACC.

The second commit fixes the exception during startup of the ACC by not initialising the executor service on the client-side.

arjantijms added some commits May 7, 2019

PAYARA-3696 Added debug option for ACC and cleaning
Signed-off-by: arjantijms <arjan.tijms@gmail.com>
PAYARA-3696 Don't init executor service when in ACC
Signed-off-by: arjantijms <arjan.tijms@gmail.com>

@arjantijms arjantijms added this to the 5.192 milestone May 7, 2019

@arjantijms arjantijms requested review from pdudits, Pandrex247 and MattGill98 May 7, 2019

@arjantijms

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

Jenkins test please

@pdudits
Copy link
Contributor

left a comment

I'm afraid this may lead to null pointer exceptions down the road at other places.
What about if the service hosted its own default config when attempts to inject it fail?

@arjantijms

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

I'm afraid this may lead to null pointer exceptions down the road at other places.

I think we'd seen those already then, as initialisation was already skipped, just with an NPE before.

@MattGill98
Copy link
Member

left a comment

LGTM. Are there tests planned to make sure this runs without exceptions?

arjantijms added some commits May 7, 2019

PAYARA-3696 Addressed review feedback - added "mock" executor
Signed-off-by: arjantijms <arjan.tijms@gmail.com>
@arjantijms

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

Jenkins test please

TimeUnit.valueOf(payaraExecutorServiceConfiguration.getThreadPoolExecutorKeepAliveTimeUnit()),
new LinkedBlockingQueue<>(threadPoolExecutorQueueSize),
(Runnable r) -> new Thread(r, "payara-executor-service-task"));
0,

This comment has been minimized.

Copy link
@MattGill98

MattGill98 May 7, 2019

Member

Is it possible to move the configured properties to local variables and override them if isAcc passes?

@pdudits

pdudits approved these changes May 8, 2019

@arjantijms arjantijms merged commit c75db44 into payara:master May 8, 2019

59 checks passed

Payara Quick Build and Test Quick build and test passed!
Details
security/snyk - api/payara-api/pom.xml (payara-ci) No new issues
Details
security/snyk - api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/admingui/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ant-tasks/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/appclient/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/batch/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/common/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/concurrent/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/connectors/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/core/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ejb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/extras/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/featuresets/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/ha/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/installer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/javaee-api/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jdbc/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/jms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/load-balancer/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/orb/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/payara-appserver-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/persistence/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/registration/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/security/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/transaction/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/web/pom.xml (payara-ci) No new issues
Details
security/snyk - appserver/webservices/pom.xml (payara-ci) No new issues
Details
security/snyk - copyright/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/admin/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/cluster/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/common/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/core/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/deployment/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/diagnostics/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/distributions/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/flashlight/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/grizzly/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/hk2/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/osgi-platforms/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/packager/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/payara-modules/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources-l10n/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/resources/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/security/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/test-utils/pom.xml (payara-ci) No new issues
Details
security/snyk - nucleus/tests/pom.xml (payara-ci) No new issues
Details
security/snyk - pom.xml (payara-ci) No new issues
Details

@arjantijms arjantijms deleted the arjantijms:PAYARA-3696-acc-executor-service branch May 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.