Accumulo
  1. Accumulo
  2. ACCUMULO-1778

Create utility for retrieving monitor location

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.6.0
    • Component/s: client
    • Labels:
      None

      Description

      If the monitor is started on a random port, it would be convenient to retrieve the location from Instance instead of having to look it up in Zookeeper.

      1. ACCUMULO-1778-2.patch
        4 kB
        Billie Rinaldi
      2. ACCUMULO-1778.patch
        7 kB
        Billie Rinaldi

        Issue Links

          Activity

          Hide
          Christopher Tubbs added a comment -

          I'm not sure I like the idea of exposing details of server configuration through the public API when it isn't needed for client operations. I'm not sure client code should have any knowledge that any monitor services even exist.

          At the very least, I'm concerned about this being a part of the public API. What if we replace the monitor with better monitoring solutions? What if the user isn't running a monitor? What if it is running multiple monitors? Would a utility class be better suited to this? Or maybe just put it in HdfsZooInstance rather than Instance itself? Will adding it to the Instance interface disrupt the API in undesirable ways?

          Is there a precedent for exposing a monitoring service within the service being monitored? It seems a bit awkward for a service to assume it's being monitored at all, never mind that it's monitored with a very specific type of monitoring tool (in this case, one that has a location attribute that it shares with the services it is monitoring).

          Show
          Christopher Tubbs added a comment - I'm not sure I like the idea of exposing details of server configuration through the public API when it isn't needed for client operations. I'm not sure client code should have any knowledge that any monitor services even exist. At the very least, I'm concerned about this being a part of the public API. What if we replace the monitor with better monitoring solutions? What if the user isn't running a monitor? What if it is running multiple monitors? Would a utility class be better suited to this? Or maybe just put it in HdfsZooInstance rather than Instance itself? Will adding it to the Instance interface disrupt the API in undesirable ways? Is there a precedent for exposing a monitoring service within the service being monitored? It seems a bit awkward for a service to assume it's being monitored at all, never mind that it's monitored with a very specific type of monitoring tool (in this case, one that has a location attribute that it shares with the services it is monitoring).
          Hide
          Billie Rinaldi added a comment -

          Here's how it would look with a utility method instead.

          The assumption I want to make is that there exists a web service that can be used to retrieve some kind of status information about Accumulo (it doesn't matter what kind) and I want to retrieve its host:port. If there is no such service, null is an acceptable response.

          Show
          Billie Rinaldi added a comment - Here's how it would look with a utility method instead. The assumption I want to make is that there exists a web service that can be used to retrieve some kind of status information about Accumulo (it doesn't matter what kind) and I want to retrieve its host:port. If there is no such service, null is an acceptable response.
          Hide
          Christopher Tubbs added a comment -

          I like the utility method better... especially if it gets put in the monitor sub-module for ACCUMULO-656 (part of ACCUMULO-658/ACCUMULO-210)

          Show
          Christopher Tubbs added a comment - I like the utility method better... especially if it gets put in the monitor sub-module for ACCUMULO-656 (part of ACCUMULO-658 / ACCUMULO-210 )
          Hide
          Billie Rinaldi added a comment -

          I like it better too. I'm not sure about putting it in a sub-module because it might be undesirable from a dependency standpoint. Unless perhaps we moved the Info class used for accumulo info to the sub-module as well. Still, Hoya for example would then have to depend on the monitor module to be able to retrieve this location. Were you also planning to move the monitor.* properties from Property? It seems like a reasonable place for this utility would be where monitor.port.client is defined.

          Show
          Billie Rinaldi added a comment - I like it better too. I'm not sure about putting it in a sub-module because it might be undesirable from a dependency standpoint. Unless perhaps we moved the Info class used for accumulo info to the sub-module as well. Still, Hoya for example would then have to depend on the monitor module to be able to retrieve this location. Were you also planning to move the monitor.* properties from Property? It seems like a reasonable place for this utility would be where monitor.port.client is defined.
          Hide
          ASF subversion and git services added a comment -

          Commit 493fc03f1cf19c1d5cdb4335d706de4aaa039234 in branch refs/heads/master from Billie Rinaldi
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=493fc03 ]

          ACCUMULO-1778 added utility for obtaining monitor location

          Show
          ASF subversion and git services added a comment - Commit 493fc03f1cf19c1d5cdb4335d706de4aaa039234 in branch refs/heads/master from Billie Rinaldi [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=493fc03 ] ACCUMULO-1778 added utility for obtaining monitor location
          Hide
          Christopher Tubbs added a comment -

          Were you also planning to move the monitor.* properties from Property?

          Not in 1.6, but I for 1.7, I was hoping to redo a lot of the way we do configuration, so individual components would have their configuration scoped more locally, instead of retrieved from a global site.xml file. For now, the Properties are remaining in the core module's configuration, which is a common dependency.

          Show
          Christopher Tubbs added a comment - Were you also planning to move the monitor.* properties from Property? Not in 1.6, but I for 1.7, I was hoping to redo a lot of the way we do configuration, so individual components would have their configuration scoped more locally, instead of retrieved from a global site.xml file. For now, the Properties are remaining in the core module's configuration, which is a common dependency.
          Hide
          John Vines added a comment -

          Can this ticket be closed?

          Show
          John Vines added a comment - Can this ticket be closed?
          Hide
          Billie Rinaldi added a comment -

          Looks okay to me.

          Show
          Billie Rinaldi added a comment - Looks okay to me.

            People

            • Assignee:
              Billie Rinaldi
              Reporter:
              Billie Rinaldi
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development