-
Notifications
You must be signed in to change notification settings - Fork 636
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SimplePool v2 #1963
Merged
Merged
SimplePool v2 #1963
+389
−249
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ClientConnection -> SimpleUndertowConnection, and SimpleClientConnectionMaker -> SimpleUndertowConnectionMaker
…onState, SimpleClientConnection -> SimpleUndertowConnection, SimpleClientConnectionMaker -> SimpleUndertowConnectionMaker, and removed the reuseConnection() method from the SimpleConnectionMaker interface. 2. Performance optimizations. 3. Numerous refactorings 4. Major update to documentation 5. Minor re-organization of connection management logic
Closed
… etc...) is not run unless debug logging is enabled
AkashWorkGit
approved these changes
Nov 8, 2023
…ance in a multithreaded environment by making its instance reference volatile and making its default constructor private
jaydeepparekh1311
approved these changes
Nov 9, 2023
Merged
Hi @stevehu , we hope you can review and merge this on Tuesday. Thanks! |
Hi @stevehu , If possible, please merge this at your earliest convenience, or please let us know if you have any concerns about this PR so we can discuss and address them. thanks! |
stevehu
pushed a commit
that referenced
this pull request
Nov 16, 2023
* refactorings: SimpleConnectionHolder -> SimpleConnectionState, SimpleClientConnection -> SimpleUndertowConnection, and SimpleClientConnectionMaker -> SimpleUndertowConnectionMaker * 1. Breaking Changes: Renamed SimpleConnectionHolder -> SimpleConnectionState, SimpleClientConnection -> SimpleUndertowConnection, SimpleClientConnectionMaker -> SimpleUndertowConnectionMaker, and removed the reuseConnection() method from the SimpleConnectionMaker interface. 2. Performance optimizations. 3. Numerous refactorings 4. Major update to documentation 5. Minor re-organization of connection management logic * fix accidental removal of fix from PR #1963 * ensure that debug logging helper code (such as loglabel() and ports() etc...) is not run unless debug logging is enabled * ensure that SimpleUndertowConnectionMaker can only have a single instance in a multithreaded environment by making its instance reference volatile and making its default constructor private * Make Correction to comments in ConsulRegistry.lookupServiceUpdate to explain how serviceUrls.size() == 0 will ensure that updateServiceCache() does not modify the service IP cache * Small formatting update --------- Co-authored-by: Michael Christoff <mike.christoff@cibc.com>
This was referenced Nov 29, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SimpleConnectionPool v2
SimpleConnectionPool
is a simple, natively mockable, threadsafe, resilient, high-performance connection pool intended to replacecom.networknt.client.http.Http2ClientConnectionPool
.Use of this connection pool fully resolves issue 1656 (#1656) by replacing the use of
Http2ClientConnection
incom.networknt.consul.client.ConsulClientImpl.lookupHealthService()
withSimpleURIConnectionPool
.SimplePool v2 also includes changes from the following issues:
Breaking Changes
SimpleConnectionHolder
renamed toSimpleConnectionState
SimpleClientConnection
renamed toSimpleUndertowConnection
SimpleClientConnectionMaker
renamed toSimpleUndertowConnectionMaker
reuseConnection()
removed from theSimpleConnectionMaker
interfaceSee issue #1905 for more details on changes.
Focus on Stability and Testability
The primary goal of SimpleConnectionPool is operational stability and reliability. Optimizations were considered only after stability and resilience were confirmed.
Easy testability was vital to confirm the correct behaviour of the connection pool under a wide array of common and corner-case conditions. To this end, the connection pool was developed with mockability built-in.
Avoidance of Consul Connection Bug
The following Consul connection-leak bug was reported in August 2020, but has yet to be resolved.
SimpleConnectionPool avoids this bug entirely by never closing connections that are in use. While this can cause connection expiry times to not be precisely honoured, it also ensures that Consul blocking queries are never canceled before they complete, thereby avoiding the conditions that manifest this bug.
Multi-Layer Architecture
The connection pool was developed using a multilayer architecure.
Level 1: SimpleConnectionPool routes requests to URI connection pools
Threads that need a connection to a URI must 'borrow' a connection from the pool and then
restore
that connection to the pool when they are done with it.A
SimpleConnectionPool
routes these URI-connection requests to aSimpleURIConnectionPool
for that URI. If aSimpleURIConnectionPool
does not exist for that URI, theSimpleConnectionPool
will create one.The
SimpleConnectionPool
needs only minimal thread synchronization since theSimpleURIConnectionPool
s are already fully thread safe. A benefit of this, is increased opportunity for concurrency. For example: N threads can request connections to N distinct URIs concurrently.Level 2: SimpleURIConnectionPools manage connections to a single URI
A
SimpleURIConnectionPool
manages connections to a single URI where the connections it manages have the ability to be used by 1 or more threads concurrently. ASimpleURIConnectionPool
:Also, note that
SimpleURIConnectionPool
s can be used used independently ofSimpleConnectionPool
. This means that, any code that only needs to connect to a single URI can use aSimpleURIConnectionPool
directly--it does not need to make requests to borrow connections from aSimpleConnectionPool
(which is only needed for code that needs to connect to multiple distinct URIs).The
SimpleURIConnectionPool
needs very little information about connections to manage the pool. It only needs to know if the connection is:Note: More than one of these properties can simultaneously apply to a connection (e.g.: an HTTP/2 connection may be both borrowed and borrowable).
Level 3: SimpleConnectionStates manage connection states
SimpleConnectionState
objects both create and manage the state of a single network connection.SimpleConnectionState
s have a 1-1 relationship with their connection. In other words:SimpleConnectionState
, andSimpleConnectionState
contains the single connection it created.When a
SimpleURIConnectionPool
needs a new connection, it will create a newSimpleConnectionState
object. The connection pool will then manage that connection exclusively by communicating through the connection's enclosingSimpleConnectionState
, rather than managing the connection directly.The
SimpleConnectionState
provides the following information about a connection to the pool:The state diagram of a
SimpleConnectionState
is shown below.SimpleConnectionState - State diagram for a connection
Level 4: SimpleConnections and SimpleConnectionMakers
SimpleConnection
interfaces wrap physical 'raw' network connections. For example, theSimpleUndertowConnection
class implementsSimpleConnection
and wraps Undertow'sClientConnection
.SimpleConnectionMaker
's are factories that createSimpleConnection
objects, and are used bySimpleConnectionState
objects to create the connection they manage. Different implementations ofSimpleConnectionMaker
can be used to create different types of raw connections. For example: ASimpleUndertowConnectionMaker
uses the Undertow networking library to create connections, aSimpleApacheConnectionMaker
could use the Apache libraries, and aSimpleMockConnectionMaker
could create mock connections used for testing.Note:
SimpleConnectionState
s only deal withSimpleConnection
objects -- they never deal directly with 'raw' connections (such as Undertow'sClientConnection
objects).Creating a new network (raw) connection
SimpleConnectionMaker
(e.g.: aSimpleUndertowConnectionMaker
)SimpleConnectState
objects (passing them aSimpleConnectionMaker
in their constructor)SimpleConnectionState
uses theSimpleConnectionMaker
to createSimpleConnection
objects that wrap raw network connectionsFinding and closing leaked connections
Typically, new connections are created in a callback thread spawned by the thread that will use the connection.
Example of connection leak detection: SimpleUndertowConnectionMaker
To create a new connection, a
SimpleUndertowConnectionMaker
will spawn a new thread to create the connection. It will then wait a configurable amount of time (a timeout) for the connection-creation thread to finish creating the connection.Below we detail 3 cases: Cases 1 and 2 detail situations in which no connection leak occurs, while case 3 details the situation where a leak does occur.
Case 1: Connection creation failed (OK)
If the connection-creation thread is unable to create the connection, then
SimpleUndertowConnectionMaker
will throw a 'connection-creation' exception andSimpleURIConnectionPool
will not recieve a new connection.Case 2: Connection successfully created before timeout (OK)
If the connection is successfully created before the timeout expires, then
SimpleUndertowConnectionMaker
will get the new connection from the connection-creation thread and return it toSimpleConnectionState
(and, in turn, the newSimpleConnectionState
holding the new connection will be returned to the pool).Case 3: Connection successfully created after timeout (!)
If the connection is successfully created after the timeout expires, then -- as in case 1 --
SimpleUndertowConnectionMaker
will throw a 'connection-creation' exception andSimpleURIConnectionPool
will not receive a new connection.However, in contrast to case 2: since a new connection was successfully created but not returned to the pool, it means that this new connection will exist but be unknown to the
SimpleURIConnectionPool
. Without knowledge of the connection,SimpleURIConnectionPool
cannot not close it and so thisuntracked
connection is considered a connection leak.Handling connections created after timeouts
Case 3 results in connections that have been created by a
SimpleConnectionMaker
but are unknown to the pool and are called 'untracked' connections (conversely, connections known to the pool are called 'tracked' connections).To ensure that all untracked connections are closed, users of a
SimpleConnectionMaker
must pass a threadsafeSet<SimpleConnection>
to theSimpleConnectionMaker
'smakeConnection()
method. Implementations ofSimpleConnectionMaker
must add all connections they create to thisSet
.To ensure all untracked connections are closed,
SimpleURIConnectionPool
will periodically remove (but not close) all tracked connections from this set (e.g.: it will remove the connections it knows about). It will then close close any connections that remain in the set, since these remaining connections are untracked. This ensures that any untracked connections in theSet
are safely closed.Mockability
Since
SimpleConnectState
objects use aSimpleConnectionMaker
to create their connections, and since the connections thatSimpleConnectionMakers
create implement theSimpleConnection
interface, it means thatSimpleURIConnectionPool
, andSimpleConnectionState
are able to manage the pool without having any knowledge of the raw connections they are managing.This means mock connections can be created by simply implementing
SimpleConnection
and creating aSimpleConnectionMaker
that instantiates these mock connections.Mock Test Harness
SimpleConnectionPool comes with a test harness that can be used to easily test mock connections. For example, to test how the connection pool will handle a connection that randomly closes can be built as follows:
1. Develop the mock connection
2. Test how the connection pool handles the connection
Plugability
SimpleConnectionPool
requires very litte information about a connection in order to manage that connection. This lends itself to supporting many different networking APIs. If one can implement aSimpleConnection
andSimpleConnectionMaker
using a networking API, then connections created using that API can be managed bySimpleConnectionPool
.ClientConnections
created using undertow's libraries can be wrapped this way. See thesimplepool.undertow
package to see how support for undertow was implemented.How to safely use SimpleConnectionPool and SimpleURIConnectionPool
The following code snippet demonstrates the recommended way to structure code that borrows a connection.
Note: The code below is the same whether
pool
is a SimpleConnectionPool or a SimpleURIConnectionPool....and this demonstrates the recommended way to borrow connections in a loop...