Repository navigation
Retry not working with SqlDataAdapter.Fill(table) (.net framework) #1591
Description
Activity
@MichaelAzzer we will look at it and will get back to you.
Hi, is there a sample project that I can use to reproduce this?
Hi, is there a sample project that I can use to reproduce this?
Yes, I prepared a simple sample to produce the problem
this zipped file contains WinForms sample with database script.sql
sample.zipReacted by Lawrence LCIThanks @MichaelAzzer , I was able to reproduce it locally with your sample project. I'll take a look and debug it.
Thanks @MichaelAzzer , I was able to reproduce it locally with your sample project. I'll take a look and debug it.
It's nice to hear that, thank you very much for your timeReacted by Lawrence LCIHi @MichaelAzzer , I noticed that the exception being thrown is a timeout exception:
Microsoft.Data.SqlClient.SqlException: 'Execution Timeout Expired. The timeout period elapsed prior to completion of the operation or the server is not responding.'I change the
command.CommandTimeout = 1;tocommand.CommandTimeout = 5;and it worked. I also tried the "working" version withSqlDataReaderand it also threw the same timeout exception until I changed the command timeout.Can you try changing the command timeout to a longer duration and try again and let me know if it works?
Hi @MichaelAzzer , I noticed that the exception being thrown is a timeout exception:
Microsoft.Data.SqlClient.SqlException: 'Execution Timeout Expired. The timeout period elapsed prior to completion of the operation or the server is not responding.'I change the
command.CommandTimeout = 1;tocommand.CommandTimeout = 5;and it worked. I also tried the "working" version withSqlDataReaderand it also threw the same timeout exception until I changed the command timeout.Can you try changing the command timeout to a longer duration and try again and let me know if it works?
Yes, you're right
I decreased command timeout intentionally to test retry but when timeout elapsed exception occurs and retry didn't work also command_Retry event didn't triggerprivate static void command_Retry(object sender, SqlRetryingEventArgs e) { SqlException ex = (SqlException)e.Exceptions[0]; MessageBox.Show(ex.Message); }Thanks for the clarification. I understand the issue now i.e. it's not triggering the retry callback from the
SqlCommand.RetryLogicProviderfromSqlDataAdapterwhere asSqlDataReaderdoes trigger the retry callback. I see thatSqlDataAdapterinherits fromDbDataAdapterwhich is where theFilllogic is implemented from theSystem.Data.Commonnamespace which I don't see source code for. TheDataTablewhere it callsLoadbelongs toSystem.Datanamespace which the implementation details is also not available, so my guess is that theDataTable.Load(IDataReader)utilizes theSqlCommand.RetryLogicProviderwhere asDbDataAdapter.Fill(DataTable)does not, which I don't think we can fix on the SqlClient side. @JRahnama correct me if I'm making the wrong assumption.After reading the implementation of SqlCommand.ExecuteReader(), I see that there's logic that checks if a RetryLogicProvider is provided in the SqlCommand and if so, the SqlReader returned is wrapped with RetryLogicProvider; however, in the core3.1 implementation of DbDataAdapter.FillInternal where the SqlDataAdapter inherits from, it grabs an IDataReader from ICommand.ExecuteReader. I think the issue is usage of the
newkeyword in theSqlCommand.ExecuteReader()which means the ICommand used in the DbDataAdapter won't be able to useRetryLogicProviderinstead it uses the one from DbCommand, which doesn't use the RetryLogicProvider. I see that @DavoudEshtehari was the last person making a change to that line. I'll double check with him to see why it was implemented this way, and if we can override the ExecuteReader so it doesn't use the DbCommand's version of it.After reading the implementation of SqlCommand.ExecuteReader(), I see that there's logic that checks if a RetryLogicProvider is provided in the SqlCommand and if so, the SqlReader returned is wrapped with RetryLogicProvider; however, in the core3.1 implementation of DbDataAdapter.FillInternal where the SqlDataAdapter inherits from, it grabs an IDataReader from ICommand.ExecuteReader. I think the issue is usage of the
newkeyword in theSqlCommand.ExecuteReader()which means the ICommand used in the DbDataAdapter won't be able to useRetryLogicProviderinstead it uses the one from DbCommand, which doesn't use the RetryLogicProvider. I see that @DavoudEshtehari was the last person making a change to that line. I'll double check with him to see why it was implemented this way, and if we can override the ExecuteReader so it doesn't use the DbCommand's version of it.I appreciate this hard work, and I want to thank 😊 you very much
Reacted by Lawrence LCIYou're welcome. After checking in with @DavoudEshtehari , he mentioned that the implementation was there since the repo was created, and the reason why we had to new was because ExecuteReader in DbCommand was not
virtualand therefore we cannot override it. Unfortunately, SqlDataAdapter won't be able to support the RetryLogicProvider, and perhaps, it should be added to the documentation, so that others are aware of this limitation.the reason why we had to new was because ExecuteReader in DbCommand was not virtual and therefore we cannot override it.
Note that ExecuteReader in DbCommand (and implements the IDbCommand.ExecuteReader discussed above) simply calls ExecuteDbDataReader, which is itself abstract. In other words, IDbCommand.ExecuteReader should indeed return an instance of SqlDataReader.
(Providers are expected to indeed replace ExecuteReader with
new, but only to return the provider-specific implementation of DbDataReader (e.g. SqlCommand.ExecuteReader returns SqlDataReader, exposing SqlClient-specific APIs). This is a general pattern in ADO.NET for implementing a sort of "covariance".)Note also that DbDataAdapter in general is a layer on top of DbCommand and DbDataReader, so at least in principle whatever works in a DbCommand implementation should also work via DbDataAdapter. There may be some additional complexity in SqlClient specifically that makes this difficult though - I'm not familiar with the internals.
Reacted by Lawrence LCISorry ,I have two more questions
1-Is this behavior in .net framework only or in both .net framework and .net core?
2-Is this problem will be fixed in the future or still one of the Microsoft.Data.SqlClient limitations?For your first question, since this behaviour was identified in your sample project, which is a .NET Framework 4.8 WinForms desktop application and DbDataAdapter from .NET Framework and DbDataAdapter from .NET Core 3.1 are nearly the same, I think the behaviour is consistent for both .NET Core and .NET Framework. As for you second question, here's my assumption and it does not reflect the actual decision on whether it'll be fixed or kept as it is. I think might be possible to fix it if we make SqlCommand inherit directly from ICommand instead of DbCommand to enable the RetryLogicProvider; however, there's a high probability that SqlCommand depends on logic from DbCommand and it may introduce some side effects that we may not expect, which is hopefully why we have unit tests, so I feel like the fix is risky, so the safer option is to say it's a limitation in Microsoft.Data.SqlClient and update the documentation, but any of the maintainers can chime in to this discussion.
@David-Engel Can you please check this and guide us to the next step?
We found that the
DataAdapterhas Fill with an overload that takes in aSqlDataReader/IDataReader; however, it is notpublici.e.protected virtualandDbDataAdapterdoes not make it public. I did a test and verified that by using the base version ofFillwith theSqlDataReaderworks with the following example by adding this in SqlDataAdapterpublic int Fill(DataTable dataTable, SqlDataReader reader) { return base.Fill(dataTable, reader); }and updating your sample code to this
using (SqlDataAdapter adapter = new SqlDataAdapter(command)) { adapter.Fill(table, command.ExecuteReader()); }and I got it to trigger the
MessageBoxfrom your callback methodconnection_RetryHence, there are 2 approaches to add
RetryLogicProvidersupport toSqlDataAdapter.Fillin MDS.- Use the example above and make a Fill(DataTabe, SqlDataReader) method in SqlDataAdapter
- Use the new keyword to hide/"override" the 5 public Fill methods from DbDataAdapter
In that case, I'll remove my PR to update the docs. @JRahnama @David-Engel what do you think?
@lcheunglci It's very nice 👌 to hear that I will try this solution
Finally I want to thank 😊 you for your hard work.Reacted by Lawrence LCII haven't been following this in detail, but FWIW hiding base methods - so as to return the more specific provider type - is a pretty standard part of how the ADO.NET API is designed: it allows for a form of covariance.
For example, DbCommand has a protected abstract ExecuteDbDataReader method, as well as a public non-virtual ExecuteReader which calls the former. Both methods are typed to return DbDataReader.
Now, an ADO.NET provider must override ExecuteDbDataReader with its implementation, which will return a provider-specific implementation of DbDataReader (e.g. SqlDataReader). But the public user AIP - ExecuteReader - still returns DbDataReader, meaning that users can't access any provider-specific APIs. So the provider hides ExecuteReader with its own version, which returns the specific type (SqlDataReader), allowing users to automatically get access to provider-specific features without downcasting etc.
(again, I have no idea how any of this is related to retries - just commenting on the method hiding part!)
Reacted by Lawrence LCI- linked a pull request that will close this issueFix | SqlDataAdapter with RetryLogicProvider #1606
on Sep 29, 2022
When use retry with command and execute command with DataReader
like
it's working
but if I used SqlDataAdapter to fill table it's not working