Uploaded image for project: 'Accumulo'
  1. Accumulo
  2. ACCUMULO-1664

Make all processes able to use random ports

    Details

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

      Description

      One method of assigning random ports (currently used by the MiniAccumuloCluster) involves finding open ports and configuring Accumulo with them. However, the selected ports could become occupied before the Accumulo processes are able to claim them.

      All the processes can be assigned random ports by setting the desired port to 0, but not all of them are then discoverable. In particular, the master, monitor and proxy would need to be addressed.

      1. ACCUMULO-1664-2.patch
        33 kB
        Billie Rinaldi
      2. ACCUMULO-1664.patch
        32 kB
        Billie Rinaldi

        Issue Links

          Activity

          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 5480ae0f1a040a39a7ac51336dd94fb3122c8863 in branch refs/heads/master from Josh Elser
          [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=5480ae0 ]

          Merge branch '1.5.1-SNAPSHOT'

          Conflicts:
          minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java

          Merging ACCUMULO-1786 is a no-op because of ACCUMULO-1664. Processes are
          able to use random ports and MAC already does in 1.6.0-SNAPSHOT, so
          there's no need to do anything special in the test.

          Show
          jira-bot ASF subversion and git services added a comment - Commit 5480ae0f1a040a39a7ac51336dd94fb3122c8863 in branch refs/heads/master from Josh Elser [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=5480ae0 ] Merge branch '1.5.1-SNAPSHOT' Conflicts: minicluster/src/test/java/org/apache/accumulo/minicluster/MiniAccumuloClusterGCTest.java Merging ACCUMULO-1786 is a no-op because of ACCUMULO-1664 . Processes are able to use random ports and MAC already does in 1.6.0-SNAPSHOT, so there's no need to do anything special in the test.
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          ACCUMULO-1664 fix ReadWriteIT's access to monitor page

          Show
          jira-bot ASF subversion and git services added a comment - Commit 132a272eba9208b881b775851e63764f84c3f46a in branch refs/heads/master from Billie Rinaldi [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=132a272 ] ACCUMULO-1664 fix ReadWriteIT's access to monitor page
          Hide
          jira-bot ASF subversion and git services added a comment -

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

          ACCUMULO-1664 made server processes able to use random ports by configuring the ports as 0

          Show
          jira-bot ASF subversion and git services added a comment - Commit 6e7269ddfffb3e35824f5de87ac93304e39d52cf in branch refs/heads/master from Billie Rinaldi [ https://git-wip-us.apache.org/repos/asf?p=accumulo.git;h=6e7269d ] ACCUMULO-1664 made server processes able to use random ports by configuring the ports as 0
          Hide
          ecn Eric Newton added a comment -

          No, the patch copies TNonblockingServerSocket.java from the thrift library and adds a method. I have posted a patch to thrift so hopefully we won't have to do that in the future.

          Show
          ecn Eric Newton added a comment - No, the patch copies TNonblockingServerSocket.java from the thrift library and adds a method. I have posted a patch to thrift so hopefully we won't have to do that in the future.
          Hide
          mdrob Mike Drob added a comment -

          Eric Newton, not sure I understand your comment. Are we hand editing generated sources now?

          Show
          mdrob Mike Drob added a comment - Eric Newton , not sure I understand your comment. Are we hand editing generated sources now?
          Hide
          billie.rinaldi Billie Rinaldi added a comment -

          Sounds good. So there isn't incorrect documentation here, I'll point out that instead of adding a new zookeeper node for the address, I added a method to ZooLock to allow rewriting the master lock data with the real address.

          Something to consider for the future is whether we want to add a method to Instance to retrieve the monitor location. For now, you can retrieve it by using the command accumulo info.

          Show
          billie.rinaldi Billie Rinaldi added a comment - Sounds good. So there isn't incorrect documentation here, I'll point out that instead of adding a new zookeeper node for the address, I added a method to ZooLock to allow rewriting the master lock data with the real address. Something to consider for the future is whether we want to add a method to Instance to retrieve the monitor location. For now, you can retrieve it by using the command accumulo info .
          Hide
          ecn Eric Newton added a comment -

          Looks good to me. I would commit it. Please add a comment to the copy/modified file that it comes with thrift, and document the one method that was added as the only change.

          Show
          ecn Eric Newton added a comment - Looks good to me. I would commit it. Please add a comment to the copy/modified file that it comes with thrift, and document the one method that was added as the only change.
          Hide
          ecn Eric Newton added a comment -

          Posted patch to thrift so that in the future we won't have to copy/modify code.

          Show
          ecn Eric Newton added a comment - Posted patch to thrift so that in the future we won't have to copy/modify code.
          Hide
          billie.rinaldi Billie Rinaldi added a comment -

          Updated patch for trunk.

          Show
          billie.rinaldi Billie Rinaldi added a comment - Updated patch for trunk.
          Hide
          billie.rinaldi Billie Rinaldi added a comment -

          Here is a first draft of a patch. Feedback is welcome. Addressing the proxy port issue was problematic, so that is not included in this patch. The proxy would have to know the instance secret for it to be able to write its location to zookeeper.

          Show
          billie.rinaldi Billie Rinaldi added a comment - Here is a first draft of a patch. Feedback is welcome. Addressing the proxy port issue was problematic, so that is not included in this patch. The proxy would have to know the instance secret for it to be able to write its location to zookeeper.
          Hide
          billie.rinaldi Billie Rinaldi added a comment -

          When the master obtains the lock, the port hasn't been assigned yet. Currently if you set the port to 0, it will say 0 in zookeeper yet the process will be listening on a random port.

          The TNonblockingServerSocket class is a simple wrapper for ServerSocketChannel. In the patch I'm experimenting with, I just copied that class and added a getPort method. I'm not sure how important it is to have the master lock obtained when it is. I was worried about moving it, so I added another zookeeper node under /master for the real address. Definitely looking for feedback on this and some other aspects of the patch.

          Show
          billie.rinaldi Billie Rinaldi added a comment - When the master obtains the lock, the port hasn't been assigned yet. Currently if you set the port to 0, it will say 0 in zookeeper yet the process will be listening on a random port. The TNonblockingServerSocket class is a simple wrapper for ServerSocketChannel. In the patch I'm experimenting with, I just copied that class and added a getPort method. I'm not sure how important it is to have the master lock obtained when it is. I was worried about moving it, so I added another zookeeper node under /master for the real address. Definitely looking for feedback on this and some other aspects of the patch.
          Hide
          ecn Eric Newton added a comment -

          Does thrift let you get the local socket address after you open the port? The master does advertise its address in its lock.

          Show
          ecn Eric Newton added a comment - Does thrift let you get the local socket address after you open the port? The master does advertise its address in its lock.

            People

            • Assignee:
              billie.rinaldi Billie Rinaldi
              Reporter:
              billie.rinaldi Billie Rinaldi
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development