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-3501 Configuration changes in MP health and metrics in Admin console don't warn about restart needed #4198

Merged
merged 7 commits into from Sep 9, 2019

Conversation

@AlanRoth
Copy link
Contributor

AlanRoth commented Sep 4, 2019

The MetricsService was not subscribed to the UnprocessedConfigListener

I removed the restart = true after every config change as UnprocessedConfigListener when not dynamic, will ask for server restart even if there are no changes when pressing save, which is expected behaviour, so restart only determined if the ActionReport message asked for restart or not, which I believe can be assumed that the message should be printed if dynamic is false, as if true, restart is not required.

AlanRoth added 3 commits Aug 22, 2019
sync
sync
@AlanRoth AlanRoth requested a review from jGauravGupta Sep 4, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Sep 4, 2019

jenkins test please

@AlanRoth AlanRoth requested a review from jbee Sep 4, 2019
@jGauravGupta

This comment has been minimized.

Copy link
Contributor

jGauravGupta commented Sep 4, 2019

Command 1:
asadmin set-metrics-configuration --enabled=true --dynamic=false

Restart server for change to take effect
Command set-metrics-configuration executed successfully.

Command 2:
asadmin get-metrics-configuration

Enabled  Dynamic  EndPoint  VirtualServers  Security Enabled  Roles
true         false        metrics                             false                     microprofile
Command get-metrics-configuration executed successfully.

Command 3:
asadmin set-metrics-configuration --enabled=true --dynamic=true

Command set-metrics-configuration executed successfully.

Command 4:
asadmin get-metrics-configuration

Enabled  Dynamic  EndPoint  VirtualServers  Security Enabled  Roles
true         true         metrics                             false                     microprofile
Command get-metrics-configuration executed successfully.

Command 5:
If by default dynamic is true then on executing command stil shows restart server warning.
asadmin set-metrics-configuration --enabled=true

Restart server for change to take effect
Command set-metrics-configuration executed successfully.
AlanRoth added 2 commits Sep 4, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Sep 4, 2019

@jGauravGupta I see the problem, I've changed it now so it will only ask for restart if getDynamic from the config is false, which means it also "remembers" the last state of dynamic. Testing it locally I'm getting the expected output now with your commands, thanks for spotting that :)

@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Sep 4, 2019

jenkins test please

Copy link
Member

Pandrex247 left a comment

I got an NPE :(

Start domain, added a random role to the roles list (bibbles) in server-config > microprofile > metrics, hit save.

Caused by: java.lang.NullPointerException
	at fish.payara.microprofile.metrics.MetricsService.changed(MetricsService.java:478)
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Sep 5, 2019

@Pandrex247 Found the issue, seems to work now

@AlanRoth AlanRoth requested a review from Pandrex247 Sep 5, 2019
@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Sep 5, 2019

jenkins test please

@AlanRoth AlanRoth removed the request for review from jbee Sep 5, 2019
@Pandrex247

This comment has been minimized.

Copy link
Member

Pandrex247 commented Sep 5, 2019

I still get the error - only on the first run though.
It seems to come from the "old" value for virtual server being null (it's trying to change null -> "")

@AlanRoth

This comment has been minimized.

Copy link
Contributor Author

AlanRoth commented Sep 6, 2019

jenkins test please

@AlanRoth AlanRoth merged commit 31700f4 into payara:master Sep 9, 2019
58 checks passed
58 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/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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.