ExecuteReader wymaga otwartego i dostępnego połączenia. Bieżący stan połączenia to Łączenie

114

Podczas próby połączenia się z bazą danych MSSQL przez ASP.NET online, otrzymam następujące informacje, gdy dwie lub więcej osób połączą się jednocześnie:

ExecuteReader wymaga otwartego i dostępnego połączenia. Bieżący stan połączenia to Łączenie.

Strona działa dobrze na moim serwerze localhost.

To jest przybliżony kod.

public Promotion retrievePromotion()
{
    int promotionID = 0;
    string promotionTitle = "";
    string promotionUrl = "";
    Promotion promotion = null;
    SqlOpenConnection();
    SqlCommand sql = SqlCommandConnection();

    sql.CommandText = "SELECT TOP 1 PromotionID, PromotionTitle, PromotionURL FROM Promotion";

    SqlDataReader dr = sql.ExecuteReader();
    while (dr.Read())
    {
        promotionID = DB2int(dr["PromotionID"]);
        promotionTitle = DB2string(dr["PromotionTitle"]);
        promotionUrl = DB2string(dr["PromotionURL"]);
        promotion = new Promotion(promotionID, promotionTitle, promotionUrl);
    }
    dr.Dispose();
    sql.Dispose();
    CloseConnection();
    return promotion;
}

Czy mogę wiedzieć, co mogło pójść nie tak i jak to naprawić?

Edycja: nie zapomnij, moje parametry połączenia i połączenie są statyczne. Myślę, że to jest powód. Proszę doradź.

public static string conString = ConfigurationManager.ConnectionStrings["dbConnection"].ConnectionString;
public static SqlConnection conn = null;
Guo Hong Lim
źródło
24
Nie używaj wspólnych / statycznych połączeń w środowisku wielowątkowym, takim jak ASP.NET, ponieważ generujesz blokady lub wyjątki (zbyt wiele otwartych połączeń itp.). Wrzuć swoją klasę DB do kosza na śmieci i twórz, otwieraj, używaj, zamykaj, usuwaj obiekty ado.net tam, gdzie ich potrzebujesz. Spójrz również na instrukcję using.
Tim Schmelter,
2
czy możesz podać mi szczegóły dotyczące SqlOpenConnection (); i sql.ExecuteReader (); funkcje? ..
ankit rajput
private void SqlOpenConnection () {try {conn = new SqlConnection (); conn.ConnectionString = conString; conn.Open (); } catch (SqlException ex) {rzut ex; }}
Guo Hong Lim
@GuoHongLim: zapomniałem wspomnieć, że nawet statyczny conStringnie dodaje nic w zakresie wydajności, ponieważ i tak jest domyślnie buforowany (jak każda wartość konfiguracyjna dla bieżącej aplikacji).
Tim Schmelter
... i żeby było to znane-nieznane: zapewnienie, że obsługa transakcji / jednostki pracy w bazie danych jest poprawne, pozostaje ćwiczeniem dla czytelnika.
mwardm

Odpowiedzi:

227

Przepraszam, że przede wszystkim komentuję, ale publikuję prawie codziennie podobny komentarz, ponieważ wiele osób uważa, że ​​rozsądnie byłoby zawrzeć funkcjonalność ADO.NET w klasie DB (ja też 10 lat temu). Przeważnie decydują się na użycie obiektów statycznych / współdzielonych, ponieważ wydaje się to być szybsze niż tworzenie nowego obiektu dla dowolnej akcji.

Nie jest to ani dobry pomysł, ani z punktu widzenia wydajności, ani bezpieczeństwa.

Nie kłusuj na terytorium puli połączeń

Istnieje dobry powód, dla którego ADO.NET wewnętrznie zarządza podstawowymi połączeniami z systemem DBMS w puli połączeń ADO-NET :

W praktyce większość aplikacji używa tylko jednej lub kilku różnych konfiguracji połączeń. Oznacza to, że podczas wykonywania aplikacji wiele identycznych połączeń będzie wielokrotnie otwieranych i zamykanych. Aby zminimalizować koszt otwierania połączeń, ADO.NET wykorzystuje technikę optymalizacji zwaną pulą połączeń.

Pule połączeń zmniejszają liczbę otwieranych nowych połączeń. Pooler zachowuje własność fizycznego połączenia. Zarządza połączeniami, utrzymując zestaw aktywnych połączeń dla każdej podanej konfiguracji połączenia. Za każdym razem, gdy użytkownik wywołuje Open przy połączeniu, osoba pulaująca szuka dostępnego połączenia w puli. Jeśli połączenie w puli jest dostępne, zwraca je do wywołującego zamiast otwierania nowego połączenia. Gdy aplikacja wywołuje polecenie Close w połączeniu, program buforujący zwraca je do puli zestawu aktywnych połączeń zamiast je zamykać. Gdy połączenie zostanie zwrócone do puli, jest gotowe do ponownego wykorzystania w następnym wywołaniu Open.

Więc oczywiście nie ma powodu, aby unikać tworzenia, otwierania lub zamykania połączeń, ponieważ w rzeczywistości nie są one w ogóle tworzone, otwierane ani zamykane. To jest „tylko” flaga dla puli połączeń, aby wiedzieć, kiedy połączenie może zostać ponownie użyte, czy nie. Jest to jednak bardzo ważna flaga, ponieważ jeśli połączenie jest „w użyciu” (zakłada pula połączeń), nowe połączenie fizyczne musi zostać otwarte dla DBMS, co jest bardzo kosztowne.

Więc nie uzyskujesz poprawy wydajności, ale odwrotnie. Jeśli maksymalny określony rozmiar puli (domyślnie 100) zostanie osiągnięty, wystąpią nawet wyjątki (zbyt wiele otwartych połączeń ...). Więc to nie tylko wpłynie ogromnie na wydajność, ale także będzie źródłem nieprzyjemnych błędów i (bez korzystania z Transakcji) obszarem zrzucania danych.

Jeśli używasz nawet połączeń statycznych, tworzysz blokadę dla każdego wątku próbującego uzyskać dostęp do tego obiektu. ASP.NET jest z natury środowiskiem wielowątkowym. Istnieje więc duża szansa na te blokady, co w najlepszym przypadku powoduje problemy z wydajnością. Właściwie wcześniej czy później otrzymasz wiele różnych wyjątków (na przykład Twój ExecuteReader wymaga otwartego i dostępnego połączenia ).

Wniosek :

  • W ogóle nie używaj ponownie połączeń ani żadnych obiektów ADO.NET.
  • Nie rób ich statycznymi / współdzielonymi (w VB.NET)
  • Zawsze twórz, otwieraj (w przypadku połączeń), używaj, zamykaj i usuwaj je tam, gdzie ich potrzebujesz (np. W metodzie)
  • użyj, using-statementaby usunąć i zamknąć (w przypadku połączeń) niejawnie

Dotyczy to nie tylko Connections (choć najbardziej zauważalne). Każdy obiekt implementujący IDisposablepowinien zostać usunięty (najprościej przez using-statement), tym bardziej w System.Data.SqlClientprzestrzeni nazw.

Wszystko to przemawia przeciwko niestandardowej klasie DB, która hermetyzuje i ponownie wykorzystuje wszystkie obiekty. To jest powód, dla którego skomentowałem, aby to wyrzucić. To tylko źródło problemu.


Edycja : Oto możliwa implementacja Twojej metody retrievePromotion:

public Promotion retrievePromotion(int promotionID)
{
    Promotion promo = null;
    var connectionString = System.Configuration.ConfigurationManager.ConnectionStrings["MainConnStr"].ConnectionString;
    using (SqlConnection connection = new SqlConnection(connectionString))
    {
        var queryString = "SELECT PromotionID, PromotionTitle, PromotionURL FROM Promotion WHERE PromotionID=@PromotionID";
        using (var da = new SqlDataAdapter(queryString, connection))
        {
            // you could also use a SqlDataReader instead
            // note that a DataTable does not need to be disposed since it does not implement IDisposable
            var tblPromotion = new DataTable();
            // avoid SQL-Injection
            da.SelectCommand.Parameters.Add("@PromotionID", SqlDbType.Int);
            da.SelectCommand.Parameters["@PromotionID"].Value = promotionID;
            try
            {
                connection.Open(); // not necessarily needed in this case because DataAdapter.Fill does it otherwise 
                da.Fill(tblPromotion);
                if (tblPromotion.Rows.Count != 0)
                {
                    var promoRow = tblPromotion.Rows[0];
                    promo = new Promotion()
                    {
                        promotionID    = promotionID,
                        promotionTitle = promoRow.Field<String>("PromotionTitle"),
                        promotionUrl   = promoRow.Field<String>("PromotionURL")
                    };
                }
            }
            catch (Exception ex)
            {
                // log this exception or throw it up the StackTrace
                // we do not need a finally-block to close the connection since it will be closed implicitely in an using-statement
                throw;
            }
        }
    }
    return promo;
}
Tim Schmelter
źródło
jest to naprawdę przydatne do nadania paradygmatu pracy nad połączeniem. Dzięki za to wyjaśnienie.
aminvincent
dobrze napisane, wyjaśnienie czegoś, co wiele osób przypadkowo odkrywa i chciałbym, żeby więcej ludzi o tym wiedziało. (+1)
Andrew Hill,
1
Dziękuję panu, myślę, że jest to najlepsze wyjaśnienie na ten temat, jakie kiedykolwiek czytałem, temat, który jest bardzo ważny i wielu początkujących myli się. Muszę pogratulować doskonałej umiejętności pisania.
Sasinosoft
@Tim Schmelter Jak mogę sprawić, aby moje zapytania działające w różnych wątkach wykorzystywały pojedynczą transakcję do zatwierdzania / wycofywania, korzystając z sugerowanego podejścia?
geeko
1

Kilka dni temu złapałem ten błąd.

W moim przypadku było tak, ponieważ korzystałem z transakcji na singletonie.

.Net nie działa dobrze z Singletonem, jak wspomniano powyżej.

Moje rozwiązanie było takie:

public class DbHelper : DbHelperCore
{
    public DbHelper()
    {
        Connection = null;
        Transaction = null;
    }

    public static DbHelper instance
    {
        get
        {
            if (HttpContext.Current is null)
                return new DbHelper();
            else if (HttpContext.Current.Items["dbh"] == null)
                HttpContext.Current.Items["dbh"] = new DbHelper();

            return (DbHelper)HttpContext.Current.Items["dbh"];
        }
    }

    public override void BeginTransaction()
    {
        Connection = new SqlConnection(Entity.Connection.getCon);
        if (Connection.State == System.Data.ConnectionState.Closed)
            Connection.Open();
        Transaction = Connection.BeginTransaction();
    }
}

Użyłem HttpContext.Current.Items dla mojej instancji. Ta klasa DbHelper i DbHelperCore jest moją własną klasą

Damon Abdiel
źródło