[ 
https://issues.apache.org/jira/browse/TINKERPOP-1774?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16598507#comment-16598507
 ] 

ASF GitHub Bot commented on TINKERPOP-1774:
-------------------------------------------

Github user jorgebay commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/903#discussion_r214292415
  
    --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs ---
    @@ -33,91 +34,135 @@ internal class ConnectionPool : IDisposable
         {
             private readonly ConnectionFactory _connectionFactory;
             private readonly ConcurrentBag<Connection> _connections = new 
ConcurrentBag<Connection>();
    -        private readonly object _connectionsLock = new object();
    +        private readonly AsyncAutoResetEvent _newConnectionAvailable = new 
AsyncAutoResetEvent();
    +        private readonly int _minPoolSize;
    +        private readonly int _maxPoolSize;
    +        private readonly TimeSpan _waitForConnectionTimeout;
    +        private int _nrConnections;
     
    -        public ConnectionPool(ConnectionFactory connectionFactory)
    +        public ConnectionPool(ConnectionFactory connectionFactory, 
ConnectionPoolSettings settings)
             {
                 _connectionFactory = connectionFactory;
    +            _minPoolSize = settings.MinSize;
    +            _maxPoolSize = settings.MaxSize;
    +            _waitForConnectionTimeout = settings.WaitForConnectionTimeout;
    +            PopulatePoolAsync().WaitUnwrap();
             }
     
    -        public int NrConnections { get; private set; }
    +        public int NrConnections => Interlocked.CompareExchange(ref 
_nrConnections, 0, 0);
    +
    +        private async Task PopulatePoolAsync()
    +        {
    +            var connectionCreationTasks = new 
List<Task<Connection>>(_minPoolSize);
    +            for (var i = 0; i < _minPoolSize; i++)
    +            {
    +                connectionCreationTasks.Add(CreateNewConnectionAsync());
    +            }
    +
    +            var createdConnections = await 
Task.WhenAll(connectionCreationTasks).ConfigureAwait(false);
    +            foreach (var c in createdConnections)
    +            {
    +                _connections.Add(c);
    +            }
    +
    +            Interlocked.CompareExchange(ref _nrConnections, _minPoolSize, 
0);
    +        }
     
             public async Task<IConnection> GetAvailableConnectionAsync()
             {
    -            if (!TryGetConnectionFromPool(out var connection))
    -                connection = await 
CreateNewConnectionAsync().ConfigureAwait(false);
    +            if (TryGetConnectionFromPool(out var connection))
    +                return ProxiedConnection(connection);
    +            connection = await 
AddConnectionIfUnderMaximumAsync().ConfigureAwait(false) ??
    +                         await 
WaitForConnectionAsync().ConfigureAwait(false);
    +            return ProxiedConnection(connection);
    +        }
     
    -            return new ProxyConnection(connection, AddConnectionIfOpen);
    +        private IConnection ProxiedConnection(Connection connection)
    +        {
    +            return new ProxyConnection(connection, ReturnConnectionIfOpen);
             }
     
    -        private bool TryGetConnectionFromPool(out Connection connection)
    +        private void ReturnConnectionIfOpen(Connection connection)
    +        {
    +            if (!connection.IsOpen)
    +            {
    +                ConsiderUnavailable();
    +                DefinitelyDestroyConnection(connection);
    +                return;
    +            }
    +
    +            _connections.Add(connection);
    +            _newConnectionAvailable.Set();
    +        }
    +
    +        private async Task<Connection> AddConnectionIfUnderMaximumAsync()
             {
                 while (true)
                 {
    -                connection = null;
    -                lock (_connectionsLock)
    -                {
    -                    if (_connections.IsEmpty) return false;
    -                    _connections.TryTake(out connection);
    -                }
    +                var nrOpened = Interlocked.CompareExchange(ref 
_nrConnections, 0, 0);
    +                if (nrOpened >= _maxPoolSize) return null;
     
    -                if (connection.IsOpen) return true;
    -                connection.Dispose();
    +                if (Interlocked.CompareExchange(ref _nrConnections, 
nrOpened + 1, nrOpened) == nrOpened)
    +                    break;
                 }
    +
    +            return await CreateNewConnectionAsync().ConfigureAwait(false);
             }
     
             private async Task<Connection> CreateNewConnectionAsync()
             {
    -            NrConnections++;
                 var newConnection = _connectionFactory.CreateConnection();
                 await newConnection.ConnectAsync().ConfigureAwait(false);
                 return newConnection;
             }
     
    -        private void AddConnectionIfOpen(Connection connection)
    +        private async Task<Connection> WaitForConnectionAsync()
             {
    -            if (!connection.IsOpen)
    +            var start = DateTimeOffset.Now;
    +            var remaining = _waitForConnectionTimeout;
    +            do
    --- End diff --
    
    I think that the loop doesn't make much sense now. We only have to `await` 
for a connection available once and return it or return or throw an 
`TimeoutException`.


> Gremlin .NET: Support min and max sizes in Connection pool
> ----------------------------------------------------------
>
>                 Key: TINKERPOP-1774
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-1774
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: dotnet
>    Affects Versions: 3.2.7
>            Reporter: Jorge Bay
>            Assignee: Florian Hockmann
>            Priority: Minor
>
> Similar to the java connection pool, we should limit the maximum amount of 
> connections and start with a minimum number.
> It would also a good opportunity to remove the synchronous acquisitions of 
> {{lock}} in the pool implementation.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to