0

I have a very strange problem that only occurs when the code in question is in a high load situations. My ASP.NET web client C# code calls a T-SQL stored procedure that has an OUTPUT parameter.

Under high loads, the data being returned sometimes does not make it back to the calling C# code. I have extracted all the relevant code into the example below;

Stored procedure:

CREATE PROCEDURE GetLoginName
    @LoginId BIGINT,
    @LoginName VARCHAR(50) OUTPUT
AS
   SET NOCOUNT ON

   SELECT @LoginName = LoginName
   FROM Logins
   WHERE Id = @LoginId

   SET NOCOUNT OFF
GO

Database base class:

public class DatabaseContextBase : IDisposable
{
    private SqlConnection _connection;
    private string _connectionString;
    private SqlInt32 _returnValue;
    private int _commandTimeout;
    private static int _maxDatabaseExecuteAttempts = 3;   

    private static int s_commandTimeout = 30;

    protected DBContextBase()
    {
        // try and get the connection string off the identity first...
        string connectionString = GetConnectionStringFromIdentity();

        if(connectionString != null)
        {
            ConstructionHelper(connectionString, s_commandTimeout);
        }
        else
        {
            // use the initialised static connection string, and call the other overload
            // of the constructor
            ConstructionHelper(s_connectionString, s_commandTimeout);
        }   
    }

    private void ConstructionHelper( string connectionString, int commandTimeout ) 
    {
        // store the connection string in a member var.
        _connectionString = connectionString;

        // store the timeout in a member var.
        _commandTimeout = commandTimeout;
    }

    public static string GetConnectionStringFromIdentity()
    {
        IIdentity identity = Thread.CurrentPrincipal.Identity as wxyz.Security.wxyzIdentityBase;
        string connectionString = null;

        if(identity != null)
        {
            connectionString = ((wxyz.Security.wxyzIdentityBase) identity ).ConnectionString;
        }

        return connectionString;
    }

    public void Dispose()
    {           
        if (_connection.State != ConnectionState.Closed)
        {
            _connection.Close();
        }

        _connection.Dispose();
        _connection = null;
    }

    protected void ExecuteNonQuery(SqlCommand command)
    {
        SqlConnection con = this.Connection;

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

            // don't need a try catch as this is only ever called from another method in this 
            // class which will wrap it.
            command.Connection = con;
            command.Transaction = _transaction;
            command.CommandTimeout = _commandTimeout;

            for (int currentAttempt = 1; currentAttempt == _maxDatabaseExecuteAttempts; currentAttempt++)
            {
                try
                {
                    // do it
                    command.ExecuteNonQuery();

                    // done, exit loop
                    break;
                }
                catch (SqlException sex)
                {
                    HandleDatabaseExceptions(currentAttempt, sex, command.CommandText);
                }
            }
        }   
    }

    protected void HandleDatabaseExceptions(int currentAttempt, SqlException sqlException, string sqlCommandName)
    {
        if (DataExceptionUtilities.IsDeadlockError(sqlException))
        {
            if (!this.IsInTransaction)
            {
                // Not in a transaction and a deadlock detected.
                // If we have not exceeded our maximum number of attemps, then try to execute the SQL query again.
                string deadlockMessage = string.Format("Deadlock occured in attempt {0} for '{1}':", currentAttempt, sqlCommandName);
                Logging.Write(new ErrorLogEntry(deadlockMessage, sqlException));

                if (currentAttempt == DBContextBase.MaxDatabaseExecuteAttempts)
                {
                    // This was the last attempt so throw the exception
                    throw sqlException;
                }

                // Wait for a short time before trying again
                WaitShortAmountOfTime();
            }
            else
            {
                // We're in a transaction, then the calling code needs to handle the deadlock
                string message = string.Format("Deadlock occured in transaction for '{0}':", sqlCommandName);

                throw new DataDeadlockException(message, sqlException);
            }
        }
        else if (this.IsInTransaction && DataExceptionUtilities.IsTimeoutError(sqlException))
        {
            // We're in a transaction and the calling code needs to handle the timeout 
            string message = string.Format("Timeout occured in transaction for '{0}':", sqlCommandName);

            // Raise a Deadlock exception and the calling code will rollback the transaction
            throw new DataDeadlockException(message, sqlException);
        }
        else
        {
            // Something else has gone wrong
            throw sqlException;
        }   
    }

    /// <summary>
    /// get the SqlConnection object owned by this database (already connected to db) 
    /// </summary>
    public SqlConnection Connection
    {
        get {
            // check whether we've got a connection string (from either identity or static initialise)
            if ( _connectionString == null )
            {
                throw new ArgumentNullException( "connectionString", "Connection string not set" );
            }

            if ( _connection != null )
            {
                return _connection;
            }
            else
            {
                _connection = new SqlConnection( _connectionString );
                return _connection;
            }
        }   
    }

    /// <summary>
    /// Return value from executed stored procedure
    /// </summary>
    public SqlInt32 ReturnValue
    {
        get { return _returnValue; }
        set { _returnValue = value; }
    }
}

Database access class:

public class AuthenticationDBCommands
{
    public static SqlCommand GetLoginName()
    {
        System.Data.SqlClient.SqlCommand cmd = new System.Data.SqlClient.SqlCommand("GetLoginName");
        cmd.CommandType = CommandType.StoredProcedure;

        cmd.Parameters.Add(new SqlParameter("@RETURN_VALUE", System.Data.SqlDbType.Int, 0, System.Data.ParameterDirection.ReturnValue, false, 0, 0, "RETURN_VALUE", System.Data.DataRowVersion.Current, SqlInt32.Null));

        cmd.Parameters.Add(new SqlParameter("@LoginId", SqlDbType.BigInt, 8, ParameterDirection.Input, false, 0, 0, "LoginId", DataRowVersion.Current, SqlInt64.Null));

        cmd.Parameters.Add(new SqlParameter("@LoginName", SqlDbType.VarChar, 50, ParameterDirection.InputOutput,false, 0, 0, "LoginName", DataRowVersion.Current, SqlString.Null));

        return cmd;
    }
}

public class AuthenticationDBContext : DatabaseContextBase
{
    public AuthenticationDBContext() : base()
    {
    } 

    public void GetLoginName(SqlInt64 LoginId, ref SqlString LoginName)
    {
        SqlCommand cmd = AuthenticationDBCommands.GetLoginName();
        cmd.Parameters[1].Value = LoginId;
        cmd.Parameters[2].Value = LoginName;

        base.ExecuteNonQuery(cmd);
        base.ReturnValue = (SqlInt32) cmd.Parameters[0].Value; 
        LoginName = (SqlString)(cmd.Parameters[2].Value); 
    }
}

So when it's used inside the ASP.NET web client it look like this:

protected string GetLoginName(long loginId)
{
    SqlString loginName = SqlString.Null;

    using (AuthenticationDBContext dbc = new AuthenticationDBContext())
    {
      dbc.GetLoginName(loginId, ref loginName);
    }

    return loginName.Value;
}

As you can see this is fairly standard stuff. But when the AuthenticationDBContext.GetLoginName() method is called by many different users in quick succession the loginName object is sometimes null.

When the SqlCommand.ExecuteNonQuery() fails to return any data it does not throw an exception.

I have tested the SQL and it always finds a value (I've inserted @LoginName into a table and it's never null). So the problem is happening after or in SqlCommand.ExecuteNonQuery();

As I said, this is an example of what it happening. In reality, data is not being returned for lots of different stored procedures.

It's also worth stating that other web clients that share the app pool in IIS are not affected when the web client in question is under a heavy load.

I'm using .NET 4.5 and my database is on SQL Server 2008.

Has anyone seen anything like this before? Can anyone recommend any changes?

Thanks in advance,

Matt

UPDATE. Thanks for all your comments. I have made the following change to the DatabaseContextBase class.

                private void ExecuteNonQueryImpl(SqlCommand command)
            {
                object _lockObject = new object();

                lock (_lockObject)
                {
                    SqlConnection con = this.GetConnection();
                    if (con.State != ConnectionState.Open)
                    {
                        con.Open();
                    }

                    // don't need a try catch as this is only ever called from another method in this 
                    // class which will wrap it.
                    command.Connection = con;
                    command.Transaction = _transaction;
                    command.CommandTimeout = _commandTimeout;

                    for (int currentAttempt = 1; currentAttempt <= _maxDatabaseExecuteAttempts; currentAttempt++)
                    {
                        try
                        {
                            // do it
                            command.ExecuteNonQuery();

                            // done, exit loop
                            break;
                        }
                        catch (SqlException sex)
                        {
                            HandleDatabaseExceptions(currentAttempt, sex, command.CommandText);
                        }
                    }

                    if (!this.IsInTransaction)
                    {
                        con.Close();
                    }
                }
            }

            public SqlConnection GetConnection()
            {
                if (this.IsInTransaction)
                {
                    return this.Connection;
                }
                else
                {
                    // check whether we've got a connection string (from either identity or static initialise)
                    if ( _connectionString == null )
                    {
                        string exceptionMessage = Language.Translate("DbContextNotInitialized");

                        throw new ArgumentNullException( "connectionString", exceptionMessage );
                    }

                    return new SqlConnection(_connectionString);
                }
            }

However, in a load test the data still sometimes comes back as null. The web client is not working in a transaction so a new SqlConnection object is created, opened and closed every time a call is made. (there are other areas of code which share the DatabaseContextBase class that do work in a transaction so the original connection property is needed)

I would like to mention that again that I'm confident that the store procedure is working correctly as I have inserted the @LoginName value into a table and it's never null.

Thanks, Matt

Matt
  • 21
  • 1
  • 4
  • As you are using command.executenonquery(), as it is not suitable to use executenonquery if are returning varchar value. Try using executescalar or use datareader/dataadapter and check if stored procedure returns any value. Here, you can also check for null values. – Paresh J Nov 05 '14 at 15:38
  • Also, have you tried changing your lock object to any `private object _lockObject = new Object();` attribute to your DatabaseContextBase instead of locking the `con`object? – rodrigogq Nov 05 '14 at 15:45
  • Hi Paresh. Why is SqlCommand.ExecuteNonQuery() not suitable for var char? I have many store procedure that are accessed this way, and in normal conditions (10 to 20 users) everything works fine. Thanks, Matt – Matt Nov 05 '14 at 15:47
  • Check out this post for more info: [What is the difference between ExecuteScalar, ExecuteReader and ExecuteNonQuery?](http://stackoverflow.com/questions/2974154/what-is-the-difference-between-executescalar-executereader-and-executenonquery) – McCee Nov 05 '14 at 15:49
  • Have you tried instead just doing a select and outputing the name, and grab it from the query that way, to see if it's an issue with the output parameter or it's a different issue? – Brian Mains Nov 05 '14 at 16:10
  • Hi Brian. I have re-written the stored procedure to return a dataset (a select statement). And like the above technique worked in normal conditions (10 to 20 users) but fails to return data in a heavy load situation. Thanks, Matt – Matt Nov 05 '14 at 16:17
  • @PareshJadhav, where did it suggested that executenonquery() can't be used for string type data? can you refer some authorized document? – Rahul Nov 05 '14 at 17:10
  • I don't see any reason why this would happen but can try `WITH NOLOCK` table hint in your procedure like `SELECT @LoginName = LoginName FROM Logins WITH NOLOCK` – Rahul Nov 05 '14 at 17:17
  • @Rahul and Matt: ExecuteNonQuery returns number of rows affected. Kindly check this URL: http://stackoverflow.com/questions/11619919/cmd-executenonquery-value-of-i-integer-is-going-1-from-0 – Paresh J Nov 06 '14 at 05:29
  • @Matt: Rahul is right, Use With NOLOCK in sql statement. – Paresh J Nov 06 '14 at 05:31
  • I'd be immediately suspicious around your use of connections - the system has a built in means of reusing connections in a safe way, just be you creating new `SqlConnection` objects - i'd get rid of the shared connection object and all of the associated locking and just have a shared connection *string*. (For starters, your initialization inside the `Connection` property has a race condition and under heavy load may leak some connections until they're garbage collected) – Damien_The_Unbeliever Nov 06 '14 at 08:17

1 Answers1

0

Your "for loop" definition is not correct.

for (int currentAttempt = 1; currentAttempt == _maxDatabaseExecuteAttempts; currentAttempt++)

This will initialize currentAttempt to 1, run the loop, increment currentAttempt, and then check to see if currentAttempt is equal to 3, which it isn't, and exit the loop. I think what you want is

for (int currentAttempt = 1; currentAttempt <= _maxDatabaseExecuteAttempts; currentAttempt++)
Bruce Dunwiddie
  • 2,888
  • 1
  • 15
  • 20
  • I know. I was struggling to get my code uploaded. "<=" was causing formatting problems, so I changed it to "==". Sorry for any confusion but this is not the problem. – Matt Nov 06 '14 at 09:12