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.

Sign up for GitHub

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

Jump to bottom

ISPN-13382 Register JMX for Single Port Transport #9592

Merged
merged 2 commits into from
Oct 12, 2021
Merged

ISPN-13382 Register JMX for Single Port Transport #9592

merged 2 commits into from
Oct 12, 2021

Conversation

tristantarrant
Copy link
Member

wfink
wfink reviewed Oct 12, 2021
View reviewed changes
.../main/java/org/infinispan/server/router/router/impl/singleport/SinglePortEndpointRouter.java Outdated
try {
unregisterServerMBeans();
} catch (Exception e) {
throw new CacheException(e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to log a FATAL/ERROR with (e) and throw (re) because the original issue is this and (e) is only a minor foolow up issue, also not sure whether the server will stop working anyway.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exceptions are unlikely to happen

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

Suggested change
throw new CacheException(e);
re.addSuppressed(e);

@pruivo
Copy link
Member

pruivo commented Oct 12, 2021

@tristantarrant ISPN-11385 Remove remote-command-executor from schema I think you have the wrong JIRA number in this commit.

@tristantarrant
Copy link
Member Author

@tristantarrant ISPN-11385 Remove remote-command-executor from schema I think you have the wrong JIRA number in this commit.

It was mentioned in DGSUP-32 so I included it here.

@tristantarrant
Copy link
Member Author

@pruivo I've applied your suggestion

@pruivo
Copy link
Member

pruivo commented Oct 12, 2021

@tristantarrant but https://issues.redhat.com/browse/ISPN-13385 is related to the REST API: Retrieve a payload with mutable attributes and the value of each

@tristantarrant
Copy link
Member Author

@pruivo In fact I meant https://issues.redhat.com/browse/ISPN-11385 :)

pruivo
pruivo approved these changes Oct 12, 2021
View reviewed changes
Copy link
Member

@pruivo pruivo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Waiting for CI

@tristantarrant
Copy link
Member Author

Some related failures

@tristantarrant
Copy link
Member Author

SinglePortTest doesn't have a cachemanager and the registerServerMBeans() call was failing

@pruivo pruivo merged commit d335772 into infinispan:main Oct 12, 2021
@pruivo
Copy link
Member

pruivo commented Oct 12, 2021

integrated! thanks @tristantarrant !

@tristantarrant tristantarrant deleted the ISPN-13382/single_port_jmx branch July 5, 2022 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wfink wfink wfink left review comments

@pruivo pruivo pruivo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tristantarrant @pruivo @wfink