Retrying method to call database

Question

Im making a system which should be running 24/7, with timers to control it. There are many calls to the database, and at some point, two methods are trying to open a connection, and one of them will fail. I've tried to make a retry method, so my methods would succeed. With the help from Michael S. Scherotter and Steven Sudit's methods in Better way to write retry logic without goto, does my method look like this:

        int MaxRetries = 3;
        Product pro = new Product();
        SqlConnection myCon = DBcon.getInstance().conn();

        string barcod = barcode;

        string query = string.Format("SELECT * FROM Product WHERE Barcode  = @barcode");

        for (int tries = MaxRetries; tries >= 0; tries--) //<-- 'tries' at the end, are unreachable?. 
        {
            try
            {

                myCon.Open();
                SqlCommand com = new SqlCommand(query, myCon);
                com.Parameters.AddWithValue("@barcode", barcode);
                SqlDataReader dr = com.ExecuteReader();
                if (dr.Read())
                {
                    pro.Barcode = dr.GetString(0);
                    pro.Name = dr.GetString(1);
                }

                    break;
                }
                catch (Exception ex)
                {
                    if (tries == 0)
                        Console.WriteLine("Exception: "+ex);
                        throw;

                }
                }



        myCon.Close();
        return pro;

When running the code, the program stops at the "for(.....)", and the exception: The connection was not closed. The connection's current state is open... This problem was the reason why I'm trying to make this method! If anyone knows how to resovle this problem, please write. Thanks

score 2 · Answer 1 · answered Dec 19 '11 at 11:57

2

You do

 myCon.Open();

inside the for loop, but

  myCon = DBcon.getInstance().conn();

outside of it. This way you try to open the same connection multiple times. If you want to protect against loss of DB connection you need to put both inside teh loop

answered Dec 19 '11 at 11:57

Eugen Rieck

64,175
10
70
92

score 1 · Answer 2 · answered Dec 19 '11 at 11:58

1

You should move the call to myCon.Open outside the for statement or wrap myCon.Open() checking the connection state before re-opening the connection:

if (myCon.State != ConnectionState.Open)
{
   myCon.Open();
}

answered Dec 19 '11 at 11:58

Kev Ritchie

1,599
10
16

score 0 · Answer 3 · answered Dec 19 '11 at 11:57

Have you tried adding

myCon.Close();

Into a Finally block. It looks like it is never being hit if you have an exception. I would highly recommend that you wrap the connection, command object etc in Using statements. This will ensure they are disposed of properly and the connection is closed.

score 0 · Accepted Answer · 2011-12-20T03:39:13.483

0

Edited for new information

How about using Transactions to preserve data integrity, getting on-the-fly connections for multiple access and wrapping them in Using statements to ensure connections are closed? eg

        Using (SqlConnection myCon = new SqlConnection('ConnectionString'))
        {
          myCon.Open();
          var transaction = myCon.BeginTransaction();    
          try 
          { 
            // ... do some DB stuff - build your command with SqlCommand but use your transaction and your connection
           var sqlCommand = new SqlCommand(CommandString, myCon, transaction);
           sqlCommand.Parameters.Add(new Parameter()); // Build up your params
           sqlCommand.ExecuteNonReader(); // Or whatever type of execution is best
           transaction.Commit();  // Yayy!
        } 
        catch (Exception ex) 
        { 
            transaction.RollBack();  // D'oh!
            // ... Some logging
        } 

        myCon.Close();
    }

This way even if you forget to Close the connection, it will still be done implicitly when the connection gets to the end of its Using statement.

edited Dec 20 '11 at 03:39

answered Dec 19 '11 at 12:09

at some point you understand it well, and some point not. Im already using singleton on the connection. Finally doesnt work for me, because it's not only this method, but lets say 20 methods trying continuies to connect to the database, and then at some point two methods are connecting at same time. – WildBoar Dec 19 '11 at 12:19
Hm interesting - is your system running in separate threads/instances? If not then you could replace the Singleton with a factory class and just hand out new database connections, that way no collissions? – Dec 19 '11 at 12:21
They are running in separete instances. – WildBoar Dec 19 '11 at 12:33
Are transactions an option? Then multiple connections can be put inside Using(var con = new SqlConnection(...)){} blocks, no more collissions, data integrity is preserved and access to the DB can still be done from class instances. – Dec 19 '11 at 12:43
I'm not sure how exactly to make it? – WildBoar Dec 19 '11 at 12:51
I've updated my previous suggestion to include Using() and Transactions. Hope it might help a little :) – Dec 19 '11 at 13:04
It's throwing an exception when RollBack(): The server failed to resume the transaction. Desc:36000005aa. The transaction active in this session has been committed or aborted by another session. – WildBoar Dec 19 '11 at 13:32
Are you getting a new connection each time in the Using() statement or are you still snaffling it from your Singleton? – Dec 19 '11 at 13:34
Visual studio won´t let me use it like you do, so it looks like this: SqlConnection myCon1 = new SqlConnection(connectionstring); – WildBoar Dec 19 '11 at 13:36
Really? There are more examples here: http://msdn.microsoft.com/en-US/library/system.data.sqlclient.sqlconnection(v=VS.80).aspx What error is it giving you? – Dec 19 '11 at 13:44
OMG! My Visual Studio sucks as hell! I had to rebuild the solution to get using to work.. But when running the program now, with the using, the before mentioned exception still being throwed. – WildBoar Dec 19 '11 at 13:52
Yes, C# requires you to rebuild before Intellisense will pick up new changes. You'll need to make this change at all points in your system: consistently use Using() and close your connection at the end. Did you make the change globally (I appreciate that might be a mammoth undertaking)? – Dec 19 '11 at 14:10
I'm gonna make changes for 25 methods then.. But if the rollback doesnøt work on this methods why should it on others? – WildBoar Dec 19 '11 at 14:22
All your methods need to be 'behaving', ie getting a fresh connection and wrapping that connection in a Using and closing it correctly once done. Othewise you'll sooner or later get a method colliding with this one and you'll get this error. Naturally these are just suggestions, you know your system better than I do :) – Dec 19 '11 at 14:24
Good luck, sorry for the long discussion: we seem to really have gone down the rabbit hole here :) – Dec 19 '11 at 15:05
Please note - double checked my example and there were two myCon.Open() commands, sincerely sorry for the mistake. Hopefully that'll sort it out. – Dec 20 '11 at 03:40
I found that mistake myself, and just removed one of them ;) – WildBoar Dec 20 '11 at 08:14
Just want to say thanks for your time end patience! It works! THANK YOU! – WildBoar Dec 20 '11 at 08:39

Retrying method to call database

4 Answers4