HTHou commented on code in PR #62:
URL:
https://github.com/apache/iotdb-client-csharp/pull/62#discussion_r3687977253
##########
src/Apache.IoTDB/ConcurrentClientQueue.cs:
##########
@@ -70,7 +90,7 @@ public Client Take()
bool timeout = false;
if (ClientQueue.IsEmpty)
{
- timeout = !Monitor.Wait(ClientQueue,
TimeSpan.FromSeconds(Timeout));
+ timeout = !Monitor.Wait(ClientQueue,
TimeSpan.FromMilliseconds(TimeoutInMs));
Review Comment:
[P1] Please preserve one overall deadline across wake-ups. `Return()` uses
`PulseAll`, so all waiters wake while only one can dequeue the returned client;
the others loop and call `Monitor.Wait` with the full `TimeoutInMs` again.
Under steady pool churn, an unlucky waiter can therefore exceed the configured
bound indefinitely, so the original “caller blocks forever” failure mode is
still possible under contention. A regression test with a 200 ms timeout and
repeated wake-ups currently waits about 556 ms. Please compute a
deadline/remaining duration and add a multi-waiter regression test.
##########
src/Apache.IoTDB/SessionPool.cs:
##########
@@ -372,6 +462,16 @@ public async Task<Client> Reconnect(Client originalClient
= null, CancellationTo
throw new ReconnectionFailedException("Error occurs when
reconnecting session pool. Could not connect to any server");
}
+ /// <summary>
+ /// Indicates whether this pool has been opened and not yet closed by
the caller.
+ /// </summary>
+ /// <remarks>
+ /// This reflects the lifecycle of the pool object only - it is NOT a
server-connectivity probe.
+ /// The client performs no heartbeat, so a server going down does not
flip this back to false;
+ /// it stays true until <see cref="Close"/> is called. To reason about
connectivity, use
Review Comment:
[P2] `Close()` does not always make this statement true. Once every
connection has become a vacant slot, `ClientQueue` is empty; the `foreach` in
`Close()` has zero iterations, and `_isClose = true` is only assigned inside
that loop. As a result, `Close()` returns while `IsOpen()` remains true, and
the vacant-slot path can reconnect again. Please transition the lifecycle state
independently of the queued clients and clear or disable vacant-slot recovery
when closing.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
