-
Notifications
You must be signed in to change notification settings - Fork 330
Implement pool shutdown for ChannelDbConnectionPool and harden WaitHandleDbConnectionPool shutdown #4302
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
base: main
Are you sure you want to change the base?
Implement pool shutdown for ChannelDbConnectionPool and harden WaitHandleDbConnectionPool shutdown #4302
Changes from 2 commits
6431209
b24d4fb
7ba73a8
b8d56d9
9a45a66
954679d
84873dc
6dc67ce
21f3aa4
d3a7200
751ad2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -92,6 +92,13 @@ internal sealed class ChannelDbConnectionPool : IDbConnectionPool | |
| /// Must be updated using <see cref="Interlocked"/> operations to ensure thread safety. | ||
| /// </summary> | ||
| private volatile int _isClearing; | ||
|
|
||
| /// <summary> | ||
| /// Tracks whether <see cref="Shutdown"/> has already initiated the shutdown sequence so that | ||
| /// repeated calls are observed as no-ops. Updated atomically via | ||
| /// <see cref="Interlocked.CompareExchange(ref int, int, int)"/>. | ||
| /// </summary> | ||
| private int _shutdownInitiated; | ||
| #endregion | ||
|
|
||
| /// <summary> | ||
|
|
@@ -254,21 +261,57 @@ public void ReturnInternalConnection(DbConnectionInternal connection, DbConnecti | |
| } | ||
| else | ||
| { | ||
| var written = _idleChannel.TryWrite(connection); | ||
| Debug.Assert(written, "Failed to write returning connection to the idle channel."); | ||
| if (!_idleChannel.TryWrite(connection)) | ||
| { | ||
| // The channel has been completed (pool is shutting down). Race window | ||
| // between the State check above and TryWrite: destroy instead of pooling. | ||
| RemoveConnection(connection); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public void Shutdown() | ||
| { | ||
| // No-op for now, warmup will be implemented later. | ||
| // idempotent. Compare-and-exchange ensures only one caller performs shutdown work. | ||
| if (Interlocked.CompareExchange(ref _shutdownInitiated, 1, 0) != 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| SqlClientEventSource.Log.TryPoolerTraceEvent( | ||
| "<prov.DbConnectionPool.Shutdown|RES|INFO|CPOOL> {0}", Id); | ||
|
|
||
| // FR-001: transition to ShuttingDown. After this point, ReturnInternalConnection | ||
| // routes returning connections to RemoveConnection. | ||
| State = ShuttingDown; | ||
|
|
||
| // FR-002 + FR-007: complete the channel writer so: | ||
| // - no further idle connections can be enqueued (TryWrite returns false), and | ||
| // - in-flight / future async waiters on ReadAsync fault with ChannelClosedException. | ||
| _idleChannel.Complete(); | ||
|
|
||
| // FR-003: drain remaining buffered idle connections and destroy them. The channel is | ||
| // unbounded so all already-enqueued items can be drained synchronously. | ||
| while (_idleChannel.TryRead(out DbConnectionInternal? connection)) | ||
| { | ||
| if (connection is not null) | ||
| { | ||
| RemoveConnection(connection); | ||
| } | ||
| // null sentinels are wake-up signals only; nothing to destroy. | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have we finished the shutdown process here? Should we transition to State Shutdown (which doesn't exixst yet)? What happens if anything between setting State = ShuttingDown and here throws? Should we allow a subsequent Shutdown() to try again? Should Shutdown() be idempotent? |
||
|
|
||
| /// <inheritdoc /> | ||
| public void Startup() | ||
| { | ||
| // No-op for now, warmup will be implemented later. | ||
| // This pool has no background timers today (idle timeout is enforced lazily in | ||
|
priyankatiwari08 marked this conversation as resolved.
|
||
| // IsLiveConnection on retrieval; pruning is not implemented). State is set to Running | ||
| // in the constructor, so this is currently the symmetrical counterpart of Shutdown. | ||
| // Background work (warmup, pruning timers) will be added here when introduced. | ||
| SqlClientEventSource.Log.TryPoolerTraceEvent( | ||
| "<prov.DbConnectionPool.Startup|RES|INFO|CPOOL> {0}", Id); | ||
| } | ||
|
|
||
| /// <inheritdoc /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,10 +26,19 @@ internal IdleConnectionChannel() | |
| { | ||
| var channel = Channel.CreateUnbounded<DbConnectionInternal?>(); | ||
| _reader = channel.Reader; | ||
| //TODO: the channel should be completed on pool shutdown | ||
| _writer = channel.Writer; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Marks the channel writer as complete. After completion, <see cref="TryWrite"/> | ||
| /// returns <see langword="false"/> for any future writes, and any in-flight or future | ||
| /// <see cref="ReadAsync"/> waiters will fault with <see cref="System.Threading.Channels.ChannelClosedException"/> | ||
| /// once the channel is drained. Used by the connection pool to signal shutdown. | ||
| /// </summary> | ||
| /// <returns><see langword="true"/> if this call completed the channel; otherwise <see langword="false"/> | ||
| /// (channel was already completed).</returns> | ||
| internal bool Complete() => _writer.TryComplete(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing tests for this new method. |
||
|
|
||
| /// <summary> | ||
| /// The number of non-null connections currently in the channel. | ||
| /// </summary> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.