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

Expose JMX RMI in product tests #16517

Merged
merged 2 commits into from
Mar 14, 2023
Merged

Conversation

wendigo
Copy link
Contributor

@wendigo wendigo commented Mar 13, 2023

This allows to connect to the running Trino instance using JDK Mission Control and capture Java Flight Recorder recordings.

Description

Additional context and related issues

Release notes

(x) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@wendigo wendigo force-pushed the serafin/jmx-rmi-pt branch from 6a1fbaf to 7e1da28 Compare March 13, 2023 12:37
Copy link
Contributor

@radek-kondziolka radek-kondziolka left a comment

Choose a reason for hiding this comment

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

Cool!

@sopel39 sopel39 requested a review from findepi March 13, 2023 13:23
@sopel39
Copy link
Member

sopel39 commented Mar 13, 2023

lgtm, but I would like @kokosing or @findepi to chime in

@wendigo wendigo force-pushed the serafin/jmx-rmi-pt branch from 7e1da28 to 0dc27b7 Compare March 13, 2023 14:02
@kokosing
Copy link
Member

I like it!

Copy link
Member

@findepi findepi left a comment

Choose a reason for hiding this comment

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

idea lgtm, only stylistic comments

Comment on lines +194 to +227
jmxPort = 6005;
}
else if (logicalName.equals(WORKER)) {
jmxPort = 6009;
}
else if (logicalName.startsWith(WORKER_NTH)) {
int workerNumber = parseInt(logicalName.substring(WORKER_NTH.length()));
jmxPort = 6008 + workerNumber;
Copy link
Member

Choose a reason for hiding this comment

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

why not contiguous?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good question. I've used the same base as in the exposing debugger part, shifted by 1000

wendigo added 2 commits March 13, 2023 16:55
This allows to connect to the running Trino instance using JDK Mission Control
and capture Java Flight Recorder recordings.
@wendigo wendigo force-pushed the serafin/jmx-rmi-pt branch from 0dc27b7 to fac6b9b Compare March 13, 2023 16:00
@wendigo
Copy link
Contributor Author

wendigo commented Mar 14, 2023

CI hit: #14637

@kokosing kokosing merged commit cb7ab58 into trinodb:master Mar 14, 2023
@kokosing
Copy link
Member

Thanks

@wendigo wendigo deleted the serafin/jmx-rmi-pt branch March 14, 2023 09:51
@github-actions github-actions bot added this to the 411 milestone Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants