Powinienem sprawdzić, czy coś istnieje w db i szybko zawieść, lub poczekać na wyjątek db

32

Posiadanie dwóch klas:

public class Parent 
{
    public int Id { get; set; }
    public int ChildId { get; set; }
}

public class Child { ... }

Przy przypisywaniu ChildIddo Parentpowinienem najpierw sprawdzić, czy istnieje w bazie danych, czy poczekać, aż baza danych zgłosi wyjątek?

Na przykład (przy użyciu Entity Framework Core):

UWAGA: tego rodzaju kontrole są WSZYSTKIE W INTERNECIE, nawet w oficjalnych dokumentach Microsoftu: https://docs.microsoft.com/en-us/aspnet/mvc/overview/getting-started/getting-started-with-ef-using- mvc / handling-concurrency-with-the-entity-framework-in-an-asp-net-mvc-application # zmodyfikuj kontroler departamentu, ale istnieje dodatkowa obsługa wyjątków dlaSaveChanges

należy również zauważyć, że głównym celem tego sprawdzenia było zwrócenie przyjaznego komunikatu i znanego statusu HTTP użytkownikowi interfejsu API, a nie całkowite zignorowanie wyjątków bazy danych. Jedyny wyjątek od miejsca, który zostanie zgłoszony, to wnętrze SaveChangeslub SaveChangesAsyncpołączenie ... więc nie będzie żadnego wyjątku podczas połączenia FindAsynclub Any. Jeśli więc dziecko istnieje, ale zostało usunięte wcześniej, SaveChangesAsynczostanie zgłoszony wyjątek współbieżności.

Zrobiłem to, ponieważ foreign key violationwyjątek będzie znacznie trudniejszy do sformatowania w celu wyświetlenia „Nie można znaleźć elementu podrzędnego o identyfikatorze {parent.ChildId}”.

public async Task<ActionResult<Parent>> CreateParent(Parent parent)
{
    // is this code redundant?
   // NOTE: its probably better to use Any isntead of FindAsync because FindAsync selects *, and Any selects 1
    var child = await _db.Children.FindAsync(parent.ChildId);
    if (child == null)
       return NotFound($"Child with id {parent.ChildId} could not be found.");

    _db.Parents.Add(parent);    
    await _db.SaveChangesAsync();        

    return parent;
}

przeciw:

public async Task<ActionResult<Parent>> CreateParent(Parent parent)
{
    _db.Parents.Add(parent);
    await _db.SaveChangesAsync();  // handle exception somewhere globally when child with the specified id doesn't exist...  

    return parent;
}

Drugi przykład w Postgres zgłosi 23503 foreign_key_violationbłąd: https://www.postgresql.org/docs/9.4/static/errcodes-appendix.html

Wadą obsługi wyjątków w ten sposób w ORM, takich jak EF, jest to, że będzie działać tylko z określonym zapleczem bazy danych. Jeśli kiedykolwiek chciałeś przełączyć się na serwer SQL lub coś innego, nie będzie to już działać, ponieważ kod błędu się zmieni.

Niepoprawne sformatowanie wyjątku dla użytkownika końcowego może ujawnić pewne rzeczy, których nikt poza deweloperami nie powinien widzieć.

Związane z:

https://stackoverflow.com/questions/6171588/preventing-race-condition-of-if-exists-update-else-insert-in-entity-framework

https://stackoverflow.com/questions/4189954/implementing-if-not-exists-insert-using-entity-framework-without-race-conditions

https://stackoverflow.com/questions/308905/should-there-be-a-transaction-for-read-queries

Konrad
źródło
2
Udostępnianie badań pomaga wszystkim . Powiedz nam, co próbowałeś i dlaczego nie spełnia twoich potrzeb. To pokazuje, że poświęciłeś trochę czasu, aby spróbować sobie pomóc, oszczędza nam to powtarzania oczywistych odpowiedzi, a przede wszystkim pomaga uzyskać bardziej konkretną i odpowiednią odpowiedź. Zobacz także How to Ask
gnat
5
Jak wspomnieli inni, istnieje możliwość wstawienia lub usunięcia rekordu jednocześnie z sprawdzaniem NotFound. Z tego powodu sprawdzenie najpierw wydaje się niedopuszczalnym rozwiązaniem. Jeśli martwisz się pisaniem obsługi wyjątków specyficznych dla Postgresa, które nie są przenośne dla innych backendów bazy danych, spróbuj ustrukturyzować procedurę obsługi wyjątków w taki sposób, aby podstawowa funkcjonalność mogła zostać rozszerzona o klasy specyficzne dla bazy danych (SQL, Postgres itp.)
bilard
3
Przeglądając komentarze, muszę powiedzieć: przestań myśleć w frazesach . „Błąd szybki” nie jest odosobnioną, pozbawioną kontekstu zasadą, której można lub należy przestrzegać na ślepo. Jest to ogólna zasada. Zawsze analizuj, co tak naprawdę próbujesz osiągnąć, a następnie rozważ dowolną technikę w świetle tego, czy pomaga ona osiągnąć ten cel, czy nie. „Fail fast” pomaga zapobiegać niezamierzonym skutkom ubocznym. A poza tym „szybko zawieść” naprawdę oznacza „zawieść, gdy tylko wykryje się problem”. Obie techniki zawodzą, gdy tylko zostanie wykryty problem, więc musisz spojrzeć na inne uwagi.
jpmc26,
1
@Konrad, co mają z tym wspólnego wyjątki? Przestań myśleć o warunkach rasowych jako czymś, co żyje w twoim kodzie: jest to właściwość wszechświata. Wszystko, co dotyka zasobu, którego nie kontroluje całkowicie (np. Bezpośredni dostęp do pamięci, pamięć współdzielona, ​​baza danych, interfejs API REST, system plików itp. Itp.) Więcej niż jeden raz i oczekuje, że pozostanie niezmienione, może mieć warunki wyścigowe. Heck, mamy do czynienia z tego w C, który nawet nie ma wyjątków. Po prostu nigdy nie rozgałęziaj się na stanie zasobu, którego nie kontrolujesz, jeśli przynajmniej jeden z rozgałęzień zadziała ze stanem tego zasobu.
Jared Smith
1
@DanielPryden W moim pytaniu nie powiedziałem, że nie chcę obsługiwać wyjątków w bazie danych (wiem, że wyjątki są nieuniknione). Myślę, że wiele osób źle zrozumiało. Chciałem mieć przyjazny komunikat o błędzie dla mojego interfejsu API sieci Web (dla użytkowników końcowych do przeczytania) Child with id {parent.ChildId} could not be found.. A formatowanie „Naruszenie klucza obcego” wydaje mi się gorsze w tym przypadku.
Konrad

Odpowiedzi:

3

Raczej niejasne pytanie, ale TAK , powinieneś najpierw sprawdzić, a nie tylko obsługiwać wyjątek DB.

Po pierwsze, w twoim przykładzie jesteś w warstwie danych, używając EF bezpośrednio w bazie danych, aby uruchomić SQL. Twój kod jest równoważny z uruchomieniem

select * from children where id = x
//if no results, perform logic
insert into parents (blah)

Sugerowana przez ciebie alternatywa to:

insert into parents (blah)
//if exception, perform logic

Używanie wyjątku do wykonywania logiki warunkowej jest powolne i powszechnie się nie podoba.

Masz warunki wyścigu i powinieneś skorzystać z transakcji. Ale można to w pełni zrobić za pomocą kodu.

using (var transaction = new TransactionScope())
{
    var child = await _db.Children.FindAsync(parent.ChildId);
    if (child == null) 
    {
       return NotFound($"Child with id {parent.ChildId} could not be found.");
    }

    _db.Parents.Add(parent);    
    await _db.SaveChangesAsync();        
    transaction.Complete();

    return parent;
}

Najważniejsze jest, aby zadać sobie pytanie:

„Czy spodziewasz się takiej sytuacji?”

Jeśli nie, to na pewno włóż i wyrzuć wyjątek. Ale obsługuj wyjątek jak każdy inny błąd, który może wystąpić.

Jeśli spodziewasz się, że to nastąpi, NIE jest to wyjątkowe i powinieneś sprawdzić, czy dziecko istnieje jako pierwsze, odpowiadając odpowiednim przyjaznym komunikatem, jeśli tak nie jest.

Edytuj - Wydaje się, że istnieje wiele kontrowersji. Przed głosowaniem zastanów się:

A. Co jeśli byłyby dwa ograniczenia FK. Czy opowiedziałbyś się za parsowaniem komunikatu wyjątku, aby ustalić, którego obiektu brakuje?

B. W przypadku pominięcia uruchamiana jest tylko jedna instrukcja SQL. To tylko trafienia, które powodują dodatkowy koszt drugiego zapytania.

C. Zwykle Id byłby kluczem zastępczym. Trudno wyobrazić sobie sytuację, w której ją znasz i nie jesteś pewien, czy jest na DB. Sprawdzanie byłoby dziwne. Ale co, jeśli jest to naturalny klucz wpisany przez użytkownika? To może mieć dużą szansę na nieobecność

Ewan
źródło
Komentarze nie są przeznaczone do rozszerzonej dyskusji; ta rozmowa została przeniesiona do czatu .
wałek klonowy
1
Jest to całkowicie błędne i mylące! Takie odpowiedzi dają złych specjalistów, z którymi zawsze muszę walczyć. SELECT nigdy nie blokuje tabeli, więc między SELECT a INSERT, UPDATE lub DELTE rekord może ulec zmianie. Więc jest to kiepskie, kiepskie oprogramowanie rażące i wypadek, który czeka na produkcję.
Daniel Lobo
1
@DanielLobo transakcje naprawia to
Ewan
1
przetestuj, jeśli mi nie wierzysz
Ewan
1
@yusha Mam kod tutaj
Ewan
111

Sprawdzanie niepowtarzalności, a następnie ustawienie jest antypatternem; zawsze może się zdarzyć, że identyfikator zostanie wstawiony jednocześnie między czasem sprawdzania a czasem zapisu. Bazy danych są wyposażone w mechanizmy takie jak ograniczenia i transakcje; większość języków programowania nie jest. Dlatego jeśli cenisz spójność danych, pozostaw to ekspertowi (baza danych), tj. Wstaw wstawkę i wyłap wyjątek, jeśli wystąpi.

Kilian Foth
źródło
34
sprawdzanie i niepowodzenie nie jest szybsze niż „próba” i nadzieja na najlepsze. Poprzedni oznacza 2 operacje do wdrożenia i wykonania przez Twój system i 2 przez DB, a najnowszy oznacza tylko jedną z nich. Sprawdzanie jest delegowane na serwer DB. Oznacza to także o jeden mniej przeskoku w sieci i o jedno zadanie mniej niż DB. Możemy myśleć, że jeszcze jedno zapytanie do bazy danych jest niedrogie, ale często zapominamy myśleć w dużym zakresie. Pomyśl o wysokiej współbieżności, uruchamiając zapytanie ponad sto razy. Może duplikować cały ruch do bazy danych. Jeśli to ma znaczenie, decyzja należy do Ciebie.
Laiv
6
@Konrad Moja pozycja jest taka, że ​​domyślnie poprawnym wyborem jest jedno zapytanie, które samo się nie powiedzie, i to osobne zapytanie poprzedzające lot, które ma ciężar dowodu, aby się usprawiedliwić. Jeśli chodzi o „stają się problem”: Więc z wykorzystaniem transakcji lub w inny sposób zapewnić, że są bezpieczne przed błędami ToCToU , prawda? Nie jest dla mnie oczywiste z opublikowanego kodu, że jesteś, ale jeśli tak nie jest, to już stało się problemem, tak jak tykająca bomba staje się problemem na długo przed faktycznym wybuchem.
mtraceur
4
@Konrad EF Core nie zamierza niejawnie umieszczać zarówno czeku, jak i wkładki w jednej transakcji, trzeba będzie wyraźnie o to poprosić. Bez transakcji sprawdzanie w pierwszej kolejności jest bezcelowe, ponieważ stan bazy danych może się zmieniać między sprawdzaniem i wstawianie. Nawet w przypadku transakcji nie można zapobiec zmianie bazy danych pod nogami. Kilka lat temu natrafiliśmy na problem przy użyciu EF z Oracle, gdzie chociaż db obsługuje to, Entity nie uruchamiał blokowania odczytanych rekordów w ramach transakcji, a tylko wstawka była traktowana jako transakcyjna.
Mr.Mindor,
3
„Sprawdzanie niepowtarzalności, a następnie ustawianie jest antypatternem” Nie powiedziałbym tego. Zależy to w dużej mierze od tego, czy można założyć, że nie nastąpiły żadne inne modyfikacje, oraz od tego, czy sprawdzenie daje bardziej użyteczny wynik (nawet tylko komunikat o błędzie, który w rzeczywistości coś znaczy dla czytelnika), gdy nie istnieje. Ponieważ baza danych obsługuje równoczesne żądania sieciowe, nie można zagwarantować, że nie nastąpią inne modyfikacje, ale są przypadki, gdy jest to uzasadnione założenie.
jpmc26,
5
Sprawdzanie niepowtarzalności w pierwszej kolejności nie eliminuje konieczności obsługi ewentualnych awarii. Z drugiej strony, jeżeli działanie to wymaga wykonywania kilku operacji, sprawdzając czy wszystko jest prawdopodobne, aby odnieść sukces przed rozpoczęciem którejkolwiek z nich jest często lepsze niż wykonujących czynności, które mogą prawdopodobnie muszą zostać wycofana. Przeprowadzenie wstępnych kontroli może nie ominąć wszystkich sytuacji, w których konieczne byłoby wycofanie, ale może to pomóc zmniejszyć częstotliwość takich przypadków.
supercat
38

Myślę, że to, co nazywacie „szybko zawiedzie”, a to, co nazywam, nie jest takie samo.

Mówienie bazy danych, aby dokonać zmiany i obsługa awarii, to jest szybkie. Twoja droga jest skomplikowana, powolna i niezbyt niezawodna.

Ta twoja technika nie zawodzi szybko, to „preflight”. Są czasem dobre powody, ale nie w przypadku korzystania z bazy danych.

gnasher729
źródło
1
Są przypadki, gdy potrzebujesz drugiego zapytania, gdy jedna klasa zależy od innej, więc nie masz wyboru w takich przypadkach.
Konrad,
4
Ale nie tutaj. Zapytania do bazy danych mogą być dość sprytne, więc generalnie wątpię w „brak wyboru”.
gnasher729,
1
Myślę, że zależy to również od aplikacji, jeśli utworzysz ją tylko dla kilku użytkowników, nie powinno to mieć znaczenia, a kod jest bardziej czytelny przy 2 zapytaniach.
Konrad,
21
Zakładasz, że twoja baza danych przechowuje niespójne dane. Innymi słowy, wygląda na to, że nie ufasz swojej bazie danych i spójności danych. Gdyby tak było, masz naprawdę duży problem, a twoje rozwiązanie to obejście. Rozwiązanie paliatywne, którego przeznaczeniem jest unieważnienie wcześniej niż później. Mogą zdarzyć się sytuacje, w których będziesz zmuszony zużyć DB poza swoją kontrolą i zarządzaniem. Z innych aplikacji. W takich przypadkach rozważałbym takie walidacje. W każdym razie @gnasher ma rację, twoje nie zawodzi szybko lub nie jest to, co rozumiemy jako szybkie.
Laiv
15

Zaczęło się od komentarza, ale stało się zbyt duże.

Nie, jak podają inne odpowiedzi, tego wzorca nie należy stosować. *

W przypadku systemów wykorzystujących komponenty asynchroniczne zawsze wystąpi wyścig, w którym baza danych (lub system plików lub inny system asynchroniczny) może zmieniać się między sprawdzaniem a zmianą. Kontrola tego typu po prostu nie jest niezawodnym sposobem zapobiegania typowi błędu, którego nie chcesz obsłużyć.
Co gorsza niż niewystarczająca, na pierwszy rzut oka sprawia wrażenie, że powinna zapobiec błędnemu zapisowi błędu, dając fałszywe poczucie bezpieczeństwa.

W każdym razie potrzebujesz obsługi błędów.

W komentarzach zapytałeś, co jeśli potrzebujesz danych z wielu źródeł.
Nadal nie.

Fundamentalna kwestia nie znika, jeśli to, co chcesz sprawdzić, staje się coraz bardziej skomplikowane.

Mimo to nadal potrzebujesz obsługi błędów.

Nawet jeśli ta kontrola byłaby niezawodnym sposobem uniknięcia określonego błędu, przed którym próbujesz się zabezpieczyć, nadal mogą wystąpić inne błędy. Co się stanie, jeśli stracisz połączenie z bazą danych lub zabraknie miejsca lub?

Prawdopodobnie nadal potrzebujesz innej obsługi błędów związanych z bazą danych. Obsługa tego konkretnego błędu powinna być prawdopodobnie jego niewielką częścią.

Jeśli potrzebujesz danych, aby określić, co zmienić, oczywiście będziesz musiał je skądś zebrać. (w zależności od narzędzi, których używasz, są prawdopodobnie lepsze sposoby niż oddzielne zapytania do ich gromadzenia) Jeśli podczas badania zebranych danych stwierdzisz, że nie musisz wprowadzać zmian, świetnie, nie rób zmiana. To ustalenie jest całkowicie odrębne od problemów związanych z obsługą błędów.

Mimo to nadal potrzebujesz obsługi błędów.

Wiem, że powtarzam się, ale uważam, że ważne jest, aby to wyjaśnić. Uprzątałem ten bałagan wcześniej.

W końcu zawiedzie. Kiedy się nie powiedzie, dotarcie do sedna będzie trudne i czasochłonne. Rozwiązywanie problemów wynikających z warunków wyścigu jest trudne. Nie występują one konsekwentnie, więc odtworzenie ich w izolacji będzie trudne, a nawet niemożliwe. Na początku nie wprowadziłeś prawidłowej obsługi błędów, więc prawdopodobnie nie będziesz miał wiele do zrobienia: może raport użytkownika końcowego o jakimś tajemniczym tekście (czego hej starałeś się przede wszystkim nie zobaczyć). Może ślad stosu, który wskazuje na tę funkcję, która, gdy na nią patrzysz, rażąco zaprzecza, że ​​błąd powinien być możliwy.

* Mogą istnieć uzasadnione powody biznesowe, aby przeprowadzić takie kontrole, na przykład aby zapobiec powielaniu kosztownej pracy aplikacji, ale nie jest to odpowiedni zamiennik dla prawidłowej obsługi błędów.

Mr.Mindor
źródło
2

Myślę, że warto tutaj zwrócić uwagę - jednym z powodów, dla których chcesz to zrobić, jest to, aby użytkownik mógł sformatować komunikat o błędzie.

Serdecznie polecam:

a) pokaż użytkownikowi końcowemu ten sam ogólny komunikat o błędzie dla każdego wystąpienia błędu.

b) zapisać rzeczywisty wyjątek w miejscu, do którego dostęp mają tylko programiści (jeśli jest na serwerze) lub w innym miejscu, które może zostać wysłane za pomocą narzędzi do zgłaszania błędów (jeśli wdrożony jest klient)

c) nie próbuj formatować szczegółów wyjątków błędów, które rejestrujesz, chyba że możesz dodać więcej przydatnych informacji. Nie chcesz przypadkowo „sformatować” jednej przydatnej informacji, której mógłbyś użyć do wyśledzenia problemu.


Krótko mówiąc - wyjątki są pełne bardzo przydatnych informacji technicznych. Nie powinno to być przeznaczone dla użytkownika końcowego, a użytkownik traci tę informację na własne ryzyko.

Paddy
źródło
2
„pokaż użytkownikowi końcowemu ten sam ogólny komunikat o błędzie dla każdego wystąpienia błędu”. to był główny powód, dla którego formatowanie wyjątku dla użytkownika końcowego wydaje się okropną rzeczą do zrobienia ..
Konrad
1
W każdym rozsądnym systemie baz danych powinieneś być w stanie programowo dowiedzieć się, dlaczego coś się nie udało. Parsowanie komunikatu wyjątku nie powinno być konieczne. Mówiąc bardziej ogólnie: kto mówi, że użytkownik musi w ogóle wyświetlić komunikat o błędzie? Możesz nie zaliczyć pierwszego wstawienia i ponowić próbę w pętli, aż się powiedzie (lub do pewnego limitu ponownych prób lub czasu). W rzeczywistości wycofywanie i ponawianie jest czymś, co i tak będziesz chciał wdrożyć w końcu.
Daniel Pryden,