Add async support for routing driver #203
Conversation
…lector when it is disposed.
zhenlineo
force-pushed
the
1.5-async-routing-driver
branch
from
46adebd
to
f08fcfe
Compare
| if (_statistics != null) | ||
| { | ||
| _statistics.Dispose(); | ||
| _statisticsCollector?.Unregister(_statistics); |
There was a problem hiding this comment.
Though it does not make any difference specifically with this usage scenerio, these two lines should be swapped for avoiding access to a disposed object.
| @@ -81,6 +94,7 @@ public void Dispose() | |||
| _pool = null; | |||
| } | |||
|
|
|||
| public static ConnectionPoolStatistics Read(IDictionary<string, object> dict) => new ConnectionPoolStatistics(dict); | |||
| public static ConnectionPoolStatistics Read(string name, IDictionary<string, object> dict) | |||
There was a problem hiding this comment.
Better to have it named as From or FromDictionary?
| @@ -49,7 +49,7 @@ public static T GetValue<T>(this IDictionary<string, object> dict, string key, T | |||
|
|
|||
| public static string ToContentString<K, V>(this IDictionary<K, V> dict) | |||
| { | |||
| var output = dict.Select(item => $"{{{item.Key}, {item.Value}}}"); | |||
| var output = dict.Select(item => $"{{{item.Key}, {item.Value.ValueToString()}}}"); | |||
There was a problem hiding this comment.
As an improvement, I think we should develop more readable display format (may be indented?) for statistics as it becomes hard to decompose & understand especially when output is large.
There was a problem hiding this comment.
Statistics will be reported in a dictionary. It depends on how the end user would like to print.
The real way of consuming statistics in my mind was that we will give the dictionary to a graphic server and show us all statistics in real time (every a few sec). I would suggest to not bother too much of how we internally print them out now. We are not really relying on console printout in our tests anyway.
| @@ -79,6 +80,18 @@ public IConnection Acquire(Uri uri) | |||
| return pool.Acquire(ignored); | |||
| } | |||
|
|
|||
| public async Task<IConnection> AcquireAsync(Uri uri) | |||
There was a problem hiding this comment.
async and await keywords could be avoided in this method.
| try | ||
| { | ||
| var result = await session.RunAsync(DiscoveryProcedure).ConfigureAwait(false); | ||
| await result.ReadAsync().ConfigureAwait(false); |
There was a problem hiding this comment.
It would be better to standardize this usage as
if (await result.ReadAsync().ConfigureAwait(false)) {
} else {
throw ....
}or negation of it.
There was a problem hiding this comment.
Actually this place
await result.ReadAsync().ConfigureAwait(false);
var record = result.Current();
What we really want is
await result.SingleAsync().ConfigureAwait(false);
So I added an extension class to impl this SingleAsync now :D
| @@ -130,7 +179,17 @@ public IConnection Acquire(AccessMode mode) | |||
|
|
|||
| public Task<IConnection> AcquireAsync(AccessMode mode) | |||
| { | |||
| throw new NotImplementedException(); | |||
| var task = new TaskCompletionSource<IConnection>(); | |||
There was a problem hiding this comment.
This can be simplified as
return Task.FromResult(Acquire(mode));|
|
||
| public async Task CloseAsync() | ||
| { | ||
| if (_connection != null) |
There was a problem hiding this comment.
async & await keywords could be avoided in this method, like
if (_connection != null) {
return _connection.CloseAsync();
}
return Task.CompletedTask;Added extension method SingleAsync on IStatementREsultReader
zhenlineo
force-pushed
the
1.5-async-routing-driver
branch
from
7c373f4
to
fcde293
Compare
Improved statistic collector to be able to pull data from multiple statistic provider and remove statistic provider once it is no longer valid.