[
https://issues.apache.org/jira/browse/GEODE-8404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17188721#comment-17188721
]
ASF GitHub Bot commented on GEODE-8404:
---------------------------------------
demery-pivotal opened a new pull request #5493:
URL: https://github.com/apache/geode/pull/5493
Removed the following unused methods:
- getRandomAvailableTCPPortRange(int)
- getRandomAvailableTCPPorts(int,bool)
- getRandomAvailableTCPPortRangeKeepers(int)
- getRandomAvailableTCPPortRangeKeepers(int,bool)
Removed two methods that used an ineffective, incorrect calculation to
try to distribute ports evenly across VMs:
- getRandomAvailablePortForDUnitSite()
- getRandomAvailableTCPPortsForDUnitSite()
These methods attempted to distribute ports by using the VM id as a
modulus. The intention was something like this (assuming 5 VMs):
VM 1 would get ports 20001, 20006, 20011, 20016, ...
VM 2 would get ports 20002, 20007, 20012, 20017, ...
VM 3 would get ports 20003, 20008, 20013, 20018, ...
VM 4 would get ports 20004, 20009, 20014, 20019, ...
VM 5 would get ports 20000, 20005, 20010, 20015, ...
But the actual calculation distributed ports like this:
VM 1: 20000, 20001, 20002, 20003, 20004, ...
VM 2: 20000, 20002, 20004, 20006, 20008, ...
VM 3: 20001, 20004, 20007, 20010, 20013, ...
VM 4: 20000, 20004, 20008, 20012, 20016, ...
VM 5: 20000, 20005, 20010, 20015, 20020, ...
... with lots of potential port collisions from one VM to another.
The few uses of these methods were replaced by calls to existing methods
getRandomAvailableTCPPort() and getRandomAvailableTCPPorts(), which
offer a more reliable method of distributing ports.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in
the commit message?
- [ ] Has your PR been rebased against the latest commit within the target
branch (typically `develop`)?
- [ ] Is your initial contribution a single, squashed commit?
- [ ] Does `gradlew build` run cleanly?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, check Concourse for build
issues and
submit an update to your PR as soon as possible. If you need help, please
send an
email to d...@geode.apache.org.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> Simplify port reservation in tests
> ----------------------------------
>
> Key: GEODE-8404
> URL: https://issues.apache.org/jira/browse/GEODE-8404
> Project: Geode
> Issue Type: Test
> Components: tests
> Reporter: Dale Emery
> Assignee: Dale Emery
> Priority: Major
> Labels: GeodeOperationAPI, pull-request-available
>
> {{AvailablePort}}, {{AvailablePortHelper}}, and {{UniquePortSupplier}}
> implement a variety of complex mechanisms to reserve ports for use in the
> product and in tests.
> This complexity is unnecessary in cases where the chosen port need not be
> restricted to a specified range. Most of the ports allocated for tests have
> no such range restrictions, and so can rely on the OS to allocate available
> ports simply, directly, and efficiently.
> In particular:
> {{AvailablePort}} implements two methods to reserve only those ports that are
> a multiple of a given modulus. These methods are implemented badly, so that
> each call can render many ports unavailable before finding one that satisfies
> the constraints. These methods are not used in Geode or in tests, so I will
> remove them rather than fixing them.
> {{AvailablePortHelper}} (used only in tests) attempts to reduce the number of
> unavailable ports it tests by partitioning the available ports among VMS, and
> by storing state in a global static variable. In almost all cases, this
> mechanism can be replaced by letting the OS choose available ports.
> {{UniquePortSupplier}} (used only in tests) remembers every port it allocates
> and will not allocate the same port twice. This mechanism has the fatal
> limitation that uniqueness is guaranteed only among uses of the same
> {{UniquePortSupplier}} instance. This mechanism can be replaced by letting
> the OS choose available ports.
> {{AvailablePort.Keeper}} retains a port reservation until the caller is ready
> to bind to the port. {{Keeper}}'s use within {{AvailablePort}} is
> unnecessary. Its use in tests is limited to only a few instances. I will try
> to make those instances unnecessary. If it turns out that some tests require
> holding onto a reservation beyond its "natural" ({{TIME_WAIT}}) duration, I
> will move {{Keeper}} to into the {{geode-junit}} module, near (or inside)
> {{AvailablePortHelper}}.
> Once this complexity is reduced to its necessary minimum, I will refactor
> these classes (safely, with additional tests to cover currently untested
> features) to remove duplication and make the remaining code clearer.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
