Warum verwendet der MongoDB-Java-Treiber einen Zufallszahlengenerator in einer Bedingung?

Lesezeit: 5 Minuten

Benutzer-Avatar
Monsieur

Ich habe den folgenden Code in gesehen dieses Bekenntnis zum Der Java-Verbindungstreiber von MongoDB, und es scheint zunächst eine Art Witz zu sein. Was macht der folgende Code?

if (!((_ok) ? true : (Math.random() > 0.1))) {
    return res;
}

(BEARBEITEN: der Code wurde seitdem aktualisiert diese Frage posten)

  • Welcher Teil davon verwirrt Sie?

    – Oliver Charlesworth

    30. Mai 2013 um 9:58 Uhr

  • Ich denke, es ist verwirrend. Dieser Code wird in einem Catch-Block ausgeführt!

    – Proviste

    30. Mai 2013 um 10:00 Uhr

  • @MarkoTopolnik: Ist es das? Es könnte deutlicher geschrieben werden als if (!ok || Math.random() < 0.1) (oder etwas ähnliches).

    – Oliver Charlesworth

    30. Mai 2013 um 10:01 Uhr

  • github.com/mongodb/mongo-java-driver/commit/… Sie sind nicht der Erste, siehe Kommentar zu dieser Zeile

    – msangel

    30. Mai 2013 um 10:03 Uhr

  • @msangel Diese Jungs scheinen die Logik zu kritisieren, nicht den Codierungsstil.

    – Marko Topolnik

    30. Mai 2013 um 10:06 Uhr

Benutzer-Avatar
Marco Topolnik

Nachdem ich die Geschichte dieser Linie untersucht habe, ist meine wichtigste Schlussfolgerung, dass eine inkompetente Programmierung am Werk war.

  1. Diese Zeile ist unnötig verschlungen. Die allgemeine Form

    a? true : b
    

    zum boolean a, b entspricht dem Einfachen

    a || b
    
  2. Die umgebende Negation und übermäßige Klammern verwickeln die Dinge weiter. Im Gedächtnis behalten Gesetze von De Morgan Es ist eine triviale Beobachtung, auf die dieses Stück Code hinausläuft

    if (!_ok && Math.random() <= 0.1)
      return res;
    
  3. Das begehen ursprünglich diese Logik eingeführt hatte

    if (_ok == true) {
      _logger.log( Level.WARNING , "Server seen down: " + _addr, e );
    } else if (Math.random() < 0.1) {
      _logger.log( Level.WARNING , "Server seen down: " + _addr );
    }
    

    – ein weiteres Beispiel für inkompetente Codierung, aber beachten Sie die umgekehrte Logik: hier wird das Ereignis protokolliert, wenn entweder _ok oder in 10 % der anderen Fälle, während der Code in 2. kehrt zurück 10 % der Zeiten und protokolliert 90 % der Zeiten. Das spätere Commit ruinierte also nicht nur die Übersichtlichkeit, sondern auch die Korrektheit selbst.

    Ich denke, in dem von Ihnen geposteten Code können wir tatsächlich sehen, wie der Autor beabsichtigte, das Original umzuwandeln if-then irgendwie buchstäblich in seine Negation, die für das Frühe erforderlich ist return Bedingung. Aber dann hat er es vermasselt und ein effektives “doppeltes Negativ” eingefügt, indem er das Ungleichheitszeichen umgedreht hat.

  4. Abgesehen von Problemen mit dem Codierungsstil ist stochastische Protokollierung an sich schon eine ziemlich zweifelhafte Praxis, zumal der Protokolleintrag sein eigenes eigenartiges Verhalten nicht dokumentiert. Die Absicht ist offensichtlich, Wiederholungen derselben Tatsache zu reduzieren: dass der Server derzeit ausgefallen ist. Die geeignete Lösung ist, sich nur anzumelden Änderungen des Serverzustands, und nicht jede seiner Beobachtungen, geschweige denn eine zufällige Auswahl von 10 % solcher Beobachtungen. Ja, das erfordert nur ein bisschen mehr Aufwand, also mal sehen.

Ich kann nur hoffen, dass sich all diese Beweise für Inkompetenz bei der Inspektion angesammelt haben nur drei Zeilen Codespricht nicht fair über das Projekt als Ganzes und dass diese Arbeit so schnell wie möglich bereinigt wird.

  • Außerdem scheint dies, soweit ich das beurteilen kann, der offizielle Java-Treiber der 10. Generation für MongoDB zu sein, also glaube ich, dass es mir nicht nur eine Meinung zum Java-Treiber, sondern auch eine Meinung zum Code von MongoDB gibt

    – Chris Travers

    1. Juni 2013 um 2:20 Uhr

  • Ausgezeichnete Analyse von nur wenigen Codezeilen, ich könnte es einfach in eine Interviewfrage verwandeln! Ihr vierter Punkt ist der eigentliche Schlüssel, warum mit diesem Projekt etwas grundlegend falsch läuft (die anderen könnten als unglückliche Programmierfehler abgetan werden).

    – Abel

    4. Juni 2013 um 22:22 Uhr

  • @ChrisTravers Es ist der offizielle Mongo-Java-Treiber für Mongo.

    – Assylias

    5. Juni 2013 um 0:00 Uhr

https://github.com/mongodb/mongo-java-driver/commit/d51b3648a8e1bf1a7b7886b7ceb343064c9e2225#commitcomment-3315694

Vor 11 Stunden von gareth-rees:

Vermutlich besteht die Idee darin, nur etwa 1/10 der Serverausfälle zu protokollieren (und so ein massives Spammen des Protokolls zu vermeiden), ohne die Kosten für die Wartung eines Zählers oder Timers zu verursachen. (Aber die Wartung eines Timers wäre sicherlich erschwinglich?)

  • Nicht zu pingelig, aber: 1/10 der Zeit wird es res zurückgeben, also wird es die anderen 9/10 Mal protokollieren.

    – Superlecker

    30. Mai 2013 um 10:09 Uhr


  • @Supericy Das ist definitiv keine Spitzfindigkeit. Das ist nur ein weiterer Beweis für die schrecklichen Programmierpraktiken dieser Person.

    – Anorov

    31. Mai 2013 um 14:32 Uhr

Benutzer-Avatar
tpdi

Fügen Sie ein mit minus 1 initialisiertes Klassenmitglied hinzu:

  private int logit = -1;

Führen Sie im Try-Block den Test durch:

 if( !ok && (logit = (logit + 1 ) % 10)  == 0 ) { //log error

Dabei wird immer der erste Fehler protokolliert, dann jeder zehnte nachfolgende Fehler. Logische Operatoren “kurzschließen”, sodass Logit nur bei einem tatsächlichen Fehler inkrementiert wird.

Wenn Sie den ersten und zehnten von wollen alle Fehler, unabhängig von der Verbindung, machen die Logit-Klasse statisch statt zu einem Member.

Wie bereits erwähnt, sollte dies Thread-sicher sein:

private synchronized int getLogit() {
   return (logit = (logit + 1 ) % 10);
}

Führen Sie im Try-Block den Test durch:

 if( !ok && getLogit() == 0 ) { //log error

Hinweis: Ich denke nicht, dass es eine gute Idee ist, 90 % der Fehler zu löschen.

Benutzer-Avatar
Jens Timmermann

Ich habe so etwas schon einmal gesehen.

Es gab einen Code, der bestimmte „Fragen“ beantworten konnte, die von einem anderen „Black Box“-Code stammten. Falls es sie nicht beantworten konnte, leitete es sie zu einem anderen Teil des “Black Box”-Codes weiter, der wirklich langsam war.

Daher tauchten manchmal zuvor nicht gesehene neue „Fragen“ auf, und sie tauchten in einem Stapel auf, etwa 100 von ihnen hintereinander.

Der Programmierer war mit der Funktionsweise des Programms zufrieden, aber er wollte die Software vielleicht in Zukunft verbessern, wenn möglicherweise neue Fragen entdeckt wurden.

Die Lösung bestand also darin, unbekannte Fragen zu protokollieren, aber wie sich herausstellte, gab es 1000 verschiedene. Die Protokolle wurden zu groß, und es gab keinen Vorteil, diese zu beschleunigen, da sie keine offensichtlichen Antworten hatten. Aber hin und wieder tauchte eine Menge Fragen auf, die beantwortet werden konnten.

Da die Protokolle zu groß wurden und die Protokollierung der Protokollierung der wirklich wichtigen Dinge im Wege stand, kam er zu dieser Lösung:

Loggen Sie nur zufällig 5%, dies wird die Logs bereinigen, während auf lange Sicht immer noch angezeigt wird, welche Fragen/Antworten hinzugefügt werden könnten.

Wenn also ein unbekanntes Ereignis auftritt, wird es in einer zufälligen Anzahl dieser Fälle protokolliert.

Ich denke, das ist ähnlich wie das, was Sie hier sehen.

Mir gefiel diese Arbeitsweise nicht, also habe ich diesen Code entfernt, und zwar diese Meldungen in einer anderen Datei protokolliertalso waren sie alle vorhanden, haben aber nicht die allgemeine Protokolldatei verwüstet.

1354770cookie-checkWarum verwendet der MongoDB-Java-Treiber einen Zufallszahlengenerator in einer Bedingung?

This website is using cookies to improve the user-friendliness. You agree by using the website further.

Privacy policy