> On March 21, 2017, 10:43 a.m., Jinmei Liao wrote: > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java > > Lines 159 (patched) > > <https://reviews.apache.org/r/57796/diff/1/?file=1670527#file1670527line161> > > > > Is this system proeprty set manually or by the locator/server when > > starting up the pulse service? > > Kevin Duling wrote: > It is set manually by the test. Prior to my change, Pulse only read from > `pulse.properties` off of the classpath to set the `pulse.port` value or used > the default. > > Jinmei Liao wrote: > So when a user starts a locator with jmx-manager-port other than 1099, he > still would have problem having pulse connect to this locator? > > Kevin Duling wrote: > He would have to set `pulse.port` to match `jmx-manager-port`. Pulse has > a lot of different properties, such as `pulse.host`, `pulse.embedded`, > `pulse.embedded.sqlf`, etc. They're laid out in the `pulse.properties` file. > One could argue that pulse should look for `jmx-manager-port` but then rules > will have to be defined for when someone specifies both properties. Which one > wins? > > Jinmei Liao wrote: > In the embeded case, I don't think that property file is even used.... > > Kevin Duling wrote: > Not quite correct. In the case of embedded, `pulse.host` and > `pulse.port` are ignored, but the rest of the properties are loaded and used. > I had tried to use `jmx-manager-port` before, but the properties are not set > in the system. Instead, they are passed down as a properties object and > visibility is lost once we reach `line 137` in > `ManagementAgent.startAgent()`. At this point, we turn it over to Jetty to > start the service, but the properties aren't handed off. I'm not sure they > can be without setting a system property. This is why I chose to use the > `pulse.port` parameter that Pulse normally looks for. > > Jinmei Liao wrote: > In the end, I would like to see user do not have to change a property > file in order to get the imbeded pulse to work correctly when starting a > locator on a non-default jmx port. The work flow should just be usual: > 1) in gfsh, when user do: "start locator --jmx-manager-port=2000" (or via > proeprty file) > 2) fire up a browser, type in localhost:7070/pulse and pulse should just > work.
Agreed, and tested that scenario. Works fine. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/57796/#review169587 ----------------------------------------------------------- On March 21, 2017, 2:48 p.m., Kevin Duling wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/57796/ > ----------------------------------------------------------- > > (Updated March 21, 2017, 2:48 p.m.) > > > Review request for geode, Jinmei Liao, Jared Stewart, Ken Howe, Kirk Lund, > and Patrick Rhomberg. > > > Repository: geode > > > Description > ------- > > GEODE-2671: When a locator is started with a custom jmx-manager-port, the > embedded pulse server still tries to connect to jmx using 1099 > > > Diffs > ----- > > geode-assembly/build.gradle 1900896da96afdcc7c776f0cd98a2aea6840fb1d > > geode-assembly/src/test/java/org/apache/geode/tools/pulse/PulseVerificationTest.java > 57711258fbbc73570656e14ee8f05550ae32e891 > > geode-core/src/main/java/org/apache/geode/management/internal/ManagementAgent.java > e88360ba1506f1a7b9c7df87899d5ec19abec630 > geode-pulse/build.gradle 298ae5a8a32d621defd3336c21614c588b2ac7dc > > geode-pulse/src/main/java/org/apache/geode/tools/pulse/internal/PulseAppListener.java > 5408a5651774a63c16f27722c6ff7bda25cbaaa8 > geode-pulse/src/main/resources/pulse.properties > 878bc680bbcc4369eb2d3859c6377b8942bc89d7 > > > Diff: https://reviews.apache.org/r/57796/diff/2/ > > > Testing > ------- > > precheckin restarted > > > Thanks, > > Kevin Duling > >
