[
https://issues.apache.org/jira/browse/GEODE-8404?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17250712#comment-17250712
]
ASF GitHub Bot commented on GEODE-8404:
---------------------------------------
demery-pivotal opened a new pull request #5861:
URL: https://github.com/apache/geode/pull/5861
Make nearly all tests use AvailablePortHelper instead of AvailablePort
to obtain ports. See the rationale below.
Also: Remove unused methods from AvailablePort.
Rationale:
AvailablePort is inherently risky as a source of ports for tests.
Each "get available port" method obtains candidate port numbers from the
desired range by randomly sampling with replacement. This means that
multiple calls can return the same port number if the port is not put
into use between calls.
Some tests failed intermittently because they made multiple calls to
AvailablePort, received the same port on multiple calls, and unknowingly
attempted bind multiple sockets to the same port number, resulting in a
BindException. See GEODE-6622 for examples.
AvailablePortHelper does not have this problem. It obtains candidate
port numbers round robin. After returning an available port,
AvailablePortHelper will not return that port again in that JVM until it
has tested every other port in the range for availability.
To reduce the chance of different JVMs selecting each other's ports,
AvailablePortHelper selects a random starting point for its round robin
search in each JVM.
For distributed tests, DUnit further arranges for the
AvailablePortHelper in each JVM to start its round robin search in a
distinct place, maximally distant from the starting points of all other
JVMs. Because AvailablePort selects randomly from the full port range,
it cannot benefit from this techique.
The problems caused by AvailablePort are rare, but inevitable, with a
frequency determined by the total size of the port range. An upcoming
change will make the available port range much smaller (~400 ports
instead of the current ~10000 ports), which will greatly increase the
frequency of this problem. But the problem exists now, and results in
intermittent BindExceptions.
----------------------------------------------------------------
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:
[email protected]
> Simplify port reservation in tests [PERMANENT]
> ----------------------------------------------
>
> 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)
