Bitte vereinfachen Sie diesen Code nicht

 (github.com/kubernetes)
2 Punkte von GN⁺ 2024-08-07 | 1 Kommentare | Auf WhatsApp teilen
  • Kubernetes’ pv_controller.go ist ein Controller, der das PV/PVC-Binding synchronisiert; gleich am Dateianfang wird festgelegt, ihn „nicht zu vereinfachen“ und den space shuttle style beizubehalten
  • Dieser Stil sieht zu jedem if ein passendes else vor und hält auch scheinbar offensichtliche Bedingungen in Kommentaren fest, um geprüfte Verzweigungen und Absichten im Code sichtbar zu machen
  • Im Zentrum des Designs stehen die bidirektionalen Pointer pvc.Spec.VolumeName und pv.Spec.ClaimRef; sie machen Konkurrenzsituationen, Löschungen, Änderungen durch Nutzer und gleichzeitige Bindings in einer Umgebung ohne Transaktionen wiederherstellbar handhabbar
  • Der Controller verknüpft die Überwachung von PV-/PVC-Änderungen, interne Caches, eine Single-Worker-Queue, Event-Aufzeichnung, dynamisches Provisioning und CSI-Migrationsschnittstellen, um Binding-Zustandsübergänge zu verwalten
  • Die ausführlichen Verzweigungen und Kommentare dienen dazu, fachliches Wissen über das Verhalten und den Kontext der Fehlerwiederherstellung zu bewahren; künftige Änderungen sollten daher demselben Stil folgen

Rolle und Schreibprinzipien von pv_controller.go

  • pv_controller.go ist die Implementierungsdatei des PersistentVolumeController im Kubernetes-Paket persistentvolume
  • Dieser Controller gleicht den Zustand von PersistentVolumeClaim und PersistentVolume ab
    • Cache-Controller, der Änderungen an PersistentVolume überwacht
    • Cache-Controller, der Änderungen an PersistentVolumeClaim überwacht
    • Synchronisierung des PV/PVC-Zustands auf Basis der Änderungs-Events beider Objekte
  • Der Kommentar am Dateianfang warnt wiederholt davor, diesen Code nicht zu vereinfachen
    • Der Stil heißt space shuttle style
    • Dabei erhält jede if-Anweisung ein entsprechendes else
    • Ziel ist es, abgesehen von einfachen Fehlerprüfungen alle Verzweigungen explizit zu machen
    • Auch scheinbar offensichtliches Verhalten wird kommentiert, damit Maintainer die Komplexität des Bindings nachvollziehen können

Warum der space shuttle style beibehalten wird

  • Dieser Controller ist das Ergebnis der Zusammenführung von Aufgaben, die ursprünglich auf drei Controller verteilt waren
  • Im Zuge der Vereinfachung des PV-Subsystems wurde ein Ansatz nötig, bei dem alle Bedingungen im Code explizit behandelt werden
  • Dadurch kann der Code ausschweifend wirken und viele Kommentare und Verzweigungen enthalten
  • Diese Ausführlichkeit ist ein Mittel, um fachliches Wissen und Kontext des Binding-Verhaltens im Code zu erhalten
  • Wer diese Datei ändert, muss den space shuttle style bewahren und bei Bedarf Verzweigungen und Kommentare auf dieselbe Weise ergänzen

Kerndesign: bidirektionale Pointer zwischen PV und PVC

  • Im Zentrum des Designs stehen bidirektionale Pointer zwischen PV und PVC
    • Pointer auf PVC-Seite: pvc.Spec.VolumeName
    • Pointer auf PV-Seite: pv.Spec.ClaimRef
  • Diese Bidirektionalität ist in einem System ohne Transaktionen schwer zu handhaben, ist aber nötig, um auch in Fehlersituationen korrektes Verhalten zu gewährleisten
  • Wenn eine rogue HA-Controller-Instanz eine Race Condition erzeugt, können mehrere nicht unterscheidbare Bindings entstehen, was Datenverlust möglich macht
  • Der Controller ist grundsätzlich für den Betrieb im Active-Passive-HA-Modus ausgelegt
    • Objektübergänge sind so gestaltet, dass sie auch mit Active-Active-HA funktionieren können
    • Wenn jedoch zwei aktive Controller häufig kollidieren, kann die Performance sinken

Binding-Verfahren und Wiederherstellungsbedingungen

  • Der Controller unterstützt bidirektionale pre-bound Objekte
    • Ein PVC, das ein bestimmtes PV wünscht
    • Ein PV, das für ein bestimmtes PVC reserviert ist
  • Das Binding erfolgt in zwei Schritten
    • Zuerst wird PV.Spec.ClaimRef geändert
    • Danach wird PVC.Spec.VolumeName geändert
  • Zu jedem Zeitpunkt dieses Vorgangs können PV oder PVC von Nutzern oder anderen Controllern geändert oder gelöscht werden
  • Es kann auch passieren, dass zwei oder mehr Controller gleichzeitig unterschiedliche Volumes und Claims binden wollen
  • Der Controller muss solche Konfliktsituationen wiederherstellen können

Wichtige Bestandteile der Controller-Struktur

  • PersistentVolumeController besitzt Lister, Informer-Sync-Funktionen, Kubernetes-Clients, Event-Recorder, Volume-Plugin-Manager und weitere Komponenten, die für die PV/PVC-Synchronisierung nötig sind
  • Die zuletzt bekannten PV/PVC-Versionen werden in internen Caches gespeichert
    • volumes persistentVolumeOrderedIndex
    • claims cache.Store
  • Dieser Cache spiegelt sowohl die neueste Version wider, die auf dem API-Server gespeichert wurde, als auch die Version, die über etcd-Events eingegangen ist
  • Ein einzelnes Binding kann grob vier Events erzeugen
    • volume.Spec-Update
    • volume.Status-Update
    • claim.Spec-Update
    • claim.Status-Update
  • Ohne internen Cache könnte ein Informer bei veraltetem Zustand versuchen, ein bereits abgeschlossenes Binding erneut zu korrigieren
  • Beim erneuten Schreiben auf den API-Server könnte dann ein Versionskonflikt mit dem bereits gespeicherten Objekt auftreten

Workqueue und Nebenläufigkeitsbeschränkungen

  • Der Controller besitzt separate Workqueues für die Verarbeitung von Claims und Volumes
    • claimQueue
    • volumeQueue
  • Jede Queue darf genau nur einen Worker Thread haben
  • Insbesondere syncClaim() ist nicht reentrant
  • Wenn zwei syncClaim()-Aufrufe gleichzeitig laufen, können folgende Probleme entstehen
    • Zwei unterschiedliche Claims werden an dasselbe Volume gebunden
    • Ein Claim wird an zwei Volumes gebunden
  • Der Controller kann solche Situationen durch Versionsfehler des API-Servers und eigene Prüfungen wiederherstellen, aber ein Multi-Worker-Ansatz kann die Gesamtgeschwindigkeit senken

syncClaim: Einstiegspunkt der PVC-Synchronisierung

  • syncClaim ist die zentrale Methode, die aufgerufen wird, wenn ein Claim erstellt, aktualisiert oder periodisch synchronisiert wird
  • Diese Methode unterscheidet nicht zwischen Event-Arten
  • Zuerst setzt sie am PVC die korrekte Migration-Annotation und aktualisiert es bei Bedarf auf dem API-Server
  • Danach verzweigt sie anhand des Vorhandenseins der Annotation AnnBindCompleted
    • Wenn die Annotation fehlt: syncUnboundClaim
    • Wenn die Annotation vorhanden ist: syncBoundClaim
  • Die eigentliche Verarbeitung ist der Lesbarkeit halber in Methoden für unbound und bound Claims aufgeteilt

checkVolumeSatisfyClaim: Prüfung der PV-Anforderungen

  • checkVolumeSatisfyClaim prüft, ob das angeforderte PV die Anforderungen des PVC erfüllt
  • Die Prüfbedingungen sind im Code explizit aufgelistet
    • Fehler, wenn das PV einen DeletionTimestamp hat
    • Fehler, wenn die PV-Kapazität kleiner ist als die vom PVC angeforderte Kapazität
    • Fehler, wenn storageClassName abweicht
    • Wenn das Feature Gate VolumeAttributesClass aktiviert ist, wird geprüft, ob VolumeAttributesClassName übereinstimmt
    • Wenn das Feature Gate deaktiviert ist, aber Claim oder Volume ein VolumeAttributesClassName enthält, ist das ein Fehler
    • Fehler, wenn volumeMode nicht kompatibel ist
    • Fehler, wenn der Access Mode nicht kompatibel ist
  • Wenn alle Bedingungen erfüllt sind, wird nil zurückgegeben

Event-Verarbeitung für PVCs mit verzögertem Binding

  • emitEventForUnboundDelayBindingClaim erzeugt ein Event mit Informationen für einen unbound Claim im Modus für verzögertes Binding
  • Der Standard-Reason ist WaitForFirstConsumer
  • Die Standardmeldung besagt, dass das Binding wartet, bis der erste Consumer erstellt wurde
  • Wenn es noch nicht geplante Pods gibt, die dieses PVC referenzieren, wird der Reason zu WaitForPodScheduled
    • Bei mehreren Pods werden alle Pod-Namen in die Meldung aufgenommen
    • Beim Volume Scheduling wird nur ein Pod berücksichtigt, aber da nicht bekannt ist, welcher Pod verwendet wird, werden alle Pods aufgenommen

syncUnboundClaim: Verarbeitung noch nicht gebundener PVCs

  • Wenn claim.Spec.VolumeName leer ist, hat der Nutzer kein bestimmtes PV angefordert
  • In diesem Fall prüft der Controller den Modus für verzögertes Binding des Claims und sucht mit findBestMatchForClaim das am besten passende PV
  • Wenn kein passendes PV vorhanden ist, geht er in folgender Reihenfolge vor
    • Wenn eine Standard-StorageClass zugewiesen werden kann, aktualisiert er das PVC und beendet die Synchronisierung
    • Wenn verzögertes Binding aktiv ist und noch kein Provisioning läuft, erzeugt er ein Warte-Event
    • Wenn der Claim eine StorageClass hat, versucht er über provisionClaim dynamisches Provisioning
    • Andernfalls zeichnet er ein FailedBinding-Event auf, weil weder ein verfügbares PV noch eine StorageClass vorhanden ist
  • Wenn ein passendes PV vorhanden ist, ruft er bind auf, um PV und PVC zu binden
    • Bei Erfolg zeichnet er Metriken für Provisioning + Binding auf und räumt den Timestamp-Cache auf
    • Wenn beim Speichern ein Fehler auftritt, schließt ein späteres syncClaim das Binding ab

Verarbeitung eines PVCs, das ein bestimmtes PV anfordert

  • Wenn claim.Spec.VolumeName nicht leer ist, hat der Nutzer ein bestimmtes PV angefordert
  • Wenn das angeforderte PV nicht im Cache ist, aktualisiert der Controller den PVC-Status auf Pending und versucht es später erneut
  • Wenn das angeforderte PV vorhanden ist und volume.Spec.ClaimRef fehlt, ist das PV noch nicht geclaimt
    • Mit checkVolumeSatisfyClaim werden die Anforderungen geprüft
    • Werden die Anforderungen nicht erfüllt, zeichnet er ein VolumeMismatch-Event auf und hält das PVC auf Pending
    • Werden die Anforderungen erfüllt, ruft er bind auf
  • Wenn das angeforderte PV bereits von diesem PVC geclaimt ist, ruft er bind auf, um das Binding abzuschließen
  • Wenn das angeforderte PV an einen anderen Claim gebunden ist, verfährt er wie folgt
    • Wenn der Claim keine Annotation besitzt, die zeigt, dass er vom Controller gebunden wurde, zeichnet er ein FailedBinding-Event auf und belässt ihn auf Pending
    • Wenn es so aussieht, als sei er vom Controller gebunden worden, aber an einen anderen Claim gebunden ist, gibt er für diesen „should never happen“-Zustand einen Fehler zurück

syncBoundClaim: Verarbeitung bereits gebundener PVCs

  • syncBoundClaim verarbeitet PVCs mit der Annotation AnnBindCompleted
  • Wenn bei einem bereits gebundenen Claim claim.Spec.VolumeName leer ist, ändert er den Claim-Status zu ClaimLost
    • Die Event-Meldung besagt, dass der gebundene Claim seine PV-Referenz verloren hat und die Daten des Volumes verloren sind
  • Wenn das PV, auf das der Claim zeigt, nicht existiert, ändert er ihn ebenfalls zu ClaimLost
    • Die Event-Meldung besagt, dass der gebundene Claim sein PersistentVolume verloren hat und die Daten verloren sind
  • Wenn das PV existiert, aber volume.Spec.ClaimRef fehlt, wird angenommen, dass das Volume wieder unbound ist, und erneut bind aufgerufen
  • Wenn ClaimRef.UID des PV der UID des Claims entspricht, gilt es als normal gebunden und bind wird aufgerufen
    • In den meisten Fällen ist das ein Aufruf ohne tatsächliche Arbeit
  • Wenn das PV auf einen anderen Claimant zeigt, wird die Claim-Phase auf den terminalen Zustand Lost gesetzt

syncVolume: Einstiegspunkt der PV-Synchronisierung

  • syncVolume ist die zentrale Methode, die bei Erstellung, Aktualisierung oder periodischer Synchronisierung eines Volumes aufgerufen wird
  • Event-Arten werden nicht unterschieden
  • Zuerst setzt sie am PV die korrekte Migration-Annotation und Finalizer und aktualisiert es bei Bedarf auf dem API-Server
  • Wenn volume.Spec.ClaimRef fehlt, gilt das Volume als unbenutzt und die Phase wird auf Available gesetzt
  • Wenn ClaimRef vorhanden ist, aber die UID leer ist, wird das PV als für ein bestimmtes PVC reserviert betrachtet und die Phase auf Available gesetzt
    • Dieses PVC ist noch nicht an dieses PV gebunden; der PVC-Sync übernimmt die Verarbeitung

Verarbeitung eines PV, dessen Claim nicht gefunden wurde

  • Wenn ein PV an einen Claim gebunden ist, sucht der Controller das PVC über Namespace/Name aus ClaimRef
  • Wenn das PVC nicht im Cache gefunden wird, führt er unter bestimmten Bedingungen zusätzliche Prüfungen durch
    • Erneute Prüfung im Informer-Cache
    • Erneute Prüfung auf dem API-Server
  • Bei PVs, die von einem externen PV-Provisioner oder externen PV-Binder erstellt wurden, kann es unter hoher Last sein, dass das PVC noch nicht mit dem lokalen Cache synchronisiert wurde
  • Um ein PVC nicht fälschlich zu reclaimen, wird eine doppelte Prüfung durchgeführt
  • Wenn festgestellt wird, dass der Claim nicht existiert, wird die Volume-Phase auf Released geändert und reclaimVolume ausgeführt
    • Wenn die bisherige Phase Failed war, wird sie nicht überschrieben
    • Wenn die Reclaim Policy Retain ist, wird geloggt, dass das PV auf einen nicht existierenden Claim verweist

Wenn die Verbindung zwischen PV und PVC nicht zusammenpasst

  • Wenn der Claim existiert, aber claim.Spec.VolumeName leer ist, hat das PVC noch keinen PV-Namen
  • Wenn volumeMode nicht passt, werden auf PV- und PVC-Seite VolumeMismatch-Events aufgezeichnet und syncClaim übersprungen
  • Wenn es kein Mismatch gibt, wird der Claim in die claimQueue gelegt, damit syncClaim bald aufgerufen wird
    • Dieser Ansatz beschleunigt das Binding provisionierter Volumes
  • Wenn Spec.VolumeName des Claims dem aktuellen Volume-Namen entspricht, gilt dies als normales Binding und die Volume-Phase wird auf Bound aktualisiert
  • Wenn der Claim an ein anderes Volume gebunden ist, wird je nach Situation verfahren
    • Wenn es ein dynamisch provisioniertes Volume ist und die Reclaim Policy Delete lautet, wird es als Released markiert und reclaimVolume ausgeführt
    • Wenn das Volume vom Controller gebunden wurde, wird es mit unbindVolume bereinigt
    • Wenn es ein vom Nutzer erstellter Pointer ist, bleibt er bestehen; dennoch wird unbindVolume aufgerufen, um die Phase zu aktualisieren und ClaimRef.UID zu leeren

Status-Updates und Event-Ausgabe

  • updateClaimStatus speichert den PVC-Status auf dem API-Server
    • Phasenänderung
    • Initialisierung von AccessModes, Capacity, CurrentVolumeAttributesClassName, wenn kein Volume vorhanden ist
    • Aktualisierung von Access Mode, Capacity und aktuellem Volume-Attributes-Class-Namen, wenn ein Volume vorhanden ist
  • Es gibt eine Bedingung, die Capacity nur in dem Moment aktualisiert, in dem ein Claim Bound wird
    • Der Unterschied zwischen der PVC-Dateisystemgröße und der PV-Block-Device-Größe kann beabsichtigt sein, daher wird die Capacity eines bereits gebundenen Claims nicht überschrieben
  • Wenn das Feature Gate VolumeAttributesClass aktiviert ist, wird CurrentVolumeAttributesClassName beim Wechsel von pending zu bound gesetzt
    • Danach sollten Resizer oder Admin-Overrides dies behandeln; wenn der Controller es dauerhaft setzt, kann eine Race Condition entstehen
  • updateClaimStatusWithEvent und updateVolumePhaseWithEvent geben nur dann Events aus, wenn sich Status/Phase tatsächlich geändert hat

Zuweisung der Standard-StorageClass

  • assignDefaultStorageClass sucht eine Standard-StorageClass und weist sie zu, wenn ein Claim keine Storage Class hat
  • Claims, die bereits eine Storage Class haben, werden ignoriert
  • Wenn keine Standard-Class vorhanden ist, wird nichts aktualisiert und false zurückgegeben
  • Wenn eine Standard-Class vorhanden ist, setzt die Methode den Class-Namen in claim.Spec.StorageClassName und aktualisiert den API-Server

Dateiumfang und explizite Grenzen

  • Nach den auf der GitHub-Seite angezeigten Dateimetadaten umfasst pv_controller.go 2038 Zeilen, 1864 LOC, 91 KB
  • Der bereitgestellte Text enthält nur den vorderen Teil der Datei bis zum Beginn der Funktion bindVolumeToClaim; der Rest folgt über den Raw-View-Link
  • Daher beschränkt sich diese Zusammenfassung auf die im bereitgestellten Code sichtbare Controller-Struktur, Designkommentare, zentrale Synchronisierungsverzweigungen und Status-Update-Logik

Verwandte Beiträge

1 Kommentare

 
GN⁺ 2024-08-07
Kommentare auf Hacker News

  • Ich weiß nicht, ob es seltsam ist, dass sich der Code in dieser Datei wirklich wie ganz normaler Go-Code anfühlt. Weil es Go ist, ist er ausführlich, und weil er nicht auf tiefe Abstraktionen setzt, wirkt er länger, aber der Code selbst sieht für mich typisch aus.
    Abstraktionen sind ein zweischneidiges Schwert, daher ist auch dieser Ansatz in Ordnung; ohne die Vorbemerkung hätte ich über den Schreibstil vermutlich nicht zweimal nachgedacht. Vielleicht liegt der Unterschied daran, dass ich mehr Erfahrung mit Unternehmenssoftware als mit Systemsoftware habe. Für jemanden, der regelmäßig zu Kubernetes beiträgt, mögen diese Kommentare überflüssig wirken, aber wenn Code in einem Unternehmensumfeld von Lesern in ferner Zukunft ohne Kontext gelesen wird, hätte ich bei dieser Komplexität eher noch mehr Kommentare hinzugefügt.

    • Früher fühlte sich solcher Code normal an, aber in den letzten ungefähr 10 Jahren scheinen viele Kürze höher zu bewerten als Explizitheit.
      Gerade bei so wichtigem Code bevorzuge ich Explizitheit deutlich. In meiner Laufbahn hatte ich mehrfach mit Code zu tun, der mehrere Bedingungen zusammenfasste und Kommentare wegließ, die fachlichen Kontext und Bedeutung erklärten; dadurch ließ sich nicht beurteilen, ob das aktuelle Verhalten beabsichtigt oder zufällig war. So etwas wird leicht zu Code, der Änderungen nicht robust übersteht, sondern Änderungen verhindert, und macht ihn zumindest für Nicht-Autoren schwer zu ändern. Unnötige Chestertons Zäune zu errichten, widerspricht der Wartbarkeit.

    • Dieser Kommentar wurde vermutlich hinzugefügt, nachdem jemand versucht hatte, den Code zu vereinfachen und gescheitert war – als Warnung an künftige Maintainer, es sich noch einmal zu überlegen, bevor sie dasselbe versuchen.
      Der Commit, der die Warnung hinzufügte, heißt „Add note about space-shuttle code style“[1], und der direkt davor liegende Commit war „Revert controller/volume: simplify sync logic in syncUnboundClaim“[2].

      [1] https://github.com/kubernetes/kubernetes/commit/de4d193d45f6...

      [2] https://github.com/kubernetes/kubernetes/commit/8a1baa4d64ca...

    • Ich dachte zunächst ähnlich, aber die stark verschachtelten if-Anweisungen haben mich umgestimmt. An dieser Stelle hätte ich definitiv Verzweigungen mit Early Returns gebaut.
      Es fühlt sich an, als sei bei „mach es lauffähig, mach es schnell, mach es schön“ nur der erste Schritt erledigt worden und „mach es schön“ ausgelassen worden. Beim Entwirren schwieriger Zustandsinteraktionen habe ich selbst schon solchen hässlichen, stark kommentierten Code geschrieben, aber normalerweise räume ich ihn vor dem Review etwas auf. Vielleicht wäre es einfach besser, ganz oben in der Datei ein großes Banner „Nicht versuchen, diesen Code zu vereinfachen“ anzubringen. Trotzdem ist er sicher nicht wirklich schlecht.

    • Seltsam mag es sein, aber du bist nicht allein. Für mich sieht dieser Code völlig normal aus. Für Komponenten, die mir für die Systemzuverlässigkeit wichtig erschienen, habe ich schon Code und Kommentare in genau diesem Stil geschrieben.
      Dem Trend zu „Code ohne Kommentare“ habe ich nie zugestimmt, und wenn ich Monate oder Jahre später zurückkam, waren meine eigenen Kommentare für mein zukünftiges Ich viel zu oft wertvoll. Ich kann mir kaum vorstellen, die in einer Komponente dieser Komplexität eingebettete Logik ohne solide Kommentare wieder zusammenzusetzen.

    • Insbesondere die Behauptung, zu jedem if gebe es einen entsprechenden else-Kommentar, scheint nicht zuverlässig zu stimmen. Viele der ifs ohne Entsprechung sind einfache Prüfungen wie if (err != nil) { oder andere Early Returns, aber selbst wenn man diese ausnimmt, scheint es ifs ohne Entsprechung zu geben.
      Allerdings waren nach meiner Erfahrung mit Unternehmenssoftware zusätzliche Kommentare auch nicht unbedingt zahlreich. In den Codebases gab es // end if-Kommentare wie eine Seuche, aber echte erklärende Kommentare waren selten.

  • Artikel zur Softwarequalität des Space Shuttle: https://archive.is/HX7n4
    Als Auszug: Das Erstaunliche an dieser Software ist nicht, wie viel sie leistet, sondern wie gut sie funktioniert. Sie stürzt nie ab, muss nicht neu gestartet werden, hat keine Bugs und komme dem, was Menschen erreichen können, nahe an Perfektion heran. Die letzten drei Versionen umfassten jeweils 420.000 Zeilen und hatten jeweils nur einen Fehler; in den letzten 11 Versionen zusammen gab es 17 Fehler. Ein kommerzielles Programm vergleichbarer Komplexität hätte etwa 5.000 Fehler gehabt, heißt es.

    • Ich frage mich, was genau „die letzten drei Versionen hatten jeweils 420.000 Zeilen und jeweils einen Fehler“ bedeutet. Wenn jede der drei Versionen genau einen Bug hatte, ist das nicht eine seltsame Art zu sagen, dass die ersten beiden Fixes nicht funktioniert oder neue Bugs eingeführt haben?
    • Es wäre interessant, die NASA-Methode mit der SpaceX-Methode zu vergleichen. SpaceX hat ebenfalls bemannte Missionen durchgeführt, also scheinen die Anforderungen ziemlich ähnlich zu sein.
    • 5000 / 17 ≈ 295. Ist es fair anzunehmen, dass ein kommerzielles Programm gleicher Komplexität 295-mal weniger Personenstunden gekostet hat?
    • Das Problem der Space-Shuttle-Entwicklungsmethodik ist, dass sie enorm teuer und langsam ist und trotzdem nicht 100 % bugfrei.
      Sie ist so teuer und langsam, dass es mit modernen Proof Assistants deutlich billiger und schneller wäre, die Korrektheit der Software zu beweisen – und tatsächlich sicherer. Projekte wie seL4 und CompCert zeigen, wie man es machen sollte.
    • Einer meiner Lieblingsartikel. Erstaunlich, dass ein Internetartikel von 1996 noch zugänglich ist.

  • Ich verstehe die Absicht hinter // KEEP THE SPACE SHUTTLE FLYING., aber es ist schon etwas witzig, in einem Kommentar auf ein System zu verweisen, das wegen seiner nicht gerade guten Sicherheitsbilanz nicht mehr betrieben wird.
    Werden sich die Leute in etwa 10 Jahren noch positiv an das Space Shuttle erinnern?

    • Die Sicherheitsprobleme des Space Shuttle waren größtenteils Hardwareprobleme, keine Softwareprobleme.
      In „Appendix F - Personal Observations on Reliability of Shuttle“ [0], Richard Feynmans Anhang zum Bericht über den Challenger-Unfall von 1986, heißt es:

      Zusammenfassend lässt sich sagen, dass das Computersoftware-Prüfsystem und die dahinterstehende Haltung von höchster Qualität sind. Der Prozess der allmählichen Selbsttäuschung durch das Absenken von Standards, der für die Sicherheitssysteme der Solid Rocket Booster oder der Space Shuttle Main Engines charakteristisch ist, ist hier nicht zu erkennen.

      Er hob die Qualität der Avioniksoftware ausdrücklich als Beispiel dafür hervor, dass auch große, komplexe Regierungsprojekte wie das Shuttle korrekt ingenieurmäßig umgesetzt werden können und nicht von vornherein zu niedriger Qualität und Risiko verdammt sind.

0: https://www.nasa.gov/history/rogersrep/v2appf.htm

  • Hat in weit über 100 erfolgreichen Missionen Menschen und Ausrüstung ins All gebracht und wieder nach Hause geholt. Ich sehe das auch heute noch positiv, und werde es vermutlich auch künftig tun. Im Sinne des menschlichen Fortschritts und des Nettoeffekts war es ein Erfolg.

  • Beendet wurde das Shuttle nicht wegen einer schlechten Sicherheitsbilanz, sondern wegen der Kosten und der erwarteten künftigen Verschlechterung der Sicherheit.
    Zwar starben bei den beiden Shuttle-Unglücken mehr Astronauten als bei anderen NASA-Katastrophen, aber gemessen an der Schwierigkeit dessen, was tatsächlich geleistet wurde, war die Sicherheitsbilanz wirklich erstaunlich. Der Code sieht sehr gut aus.

  • Die Situation des Space Shuttle ist komplexer, als einfach zu sagen, es sei unsicher gewesen. Nach Missionen betrachtet hat es eine bessere Bilanz als andere Trägersysteme. Beim Shuttle gab es 2 tödliche Missionen bei 135 Einsätzen, bei der sowjetischen Soyuz 2 bei 66, und SpaceShipTwo hat mit 1 tödlichen Mission bei gerade einmal 12 Flügen eine erschreckend schlechte Bilanz.
    Allerdings hatte das Space Shuttle eine Besatzungskapazität, die für die meisten Missionen weit größer war als nötig. Anders als Apollo oder Soyuz mit 3 Personen konnte es bis zu 8 Menschen befördern, und wenn man bedenkt, dass die meisten Missionen der Sowjetunion/Roscosmos, ESA und CNSA vollständig unbemannte autonome Missionen waren, gab es dort schlicht keine Crew, die in Gefahr geraten konnte. Vielleicht passt diese Analogie eher auf Kubernetes: ein hochentwickeltes, leistungsfähiges und vielseitiges System, das viel Aufmerksamkeit verlangt und vermutlich etwas häufiger eingesetzt wird, als nötig wäre.

  • Nach der gängigsten Kennzahl, nämlich pro Passagiermeile, gehört das Space Shuttle zu den sichersten Fahrzeugen, die je gebaut wurden und geflogen sind.
    Als jemand, dessen Kindheit genau in den 1980ern lag, frage ich mich ehrlich: Wie könnte man es nicht in guter Erinnerung behalten? Ist man einfach zu jung, sodass man dieses Programm und all seine Missionen und Errungenschaften nur rückblickend betrachtet und nur eine Perspektive hat, die von der heutigen Stimmung einer Ära geprägt ist, die von privaten Raumfahrtauftragnehmern dominiert wird?

  • Richard Hipps Bericht darüber, wie SQLite-Code an Luftfahrtstandards angepasst wurde, ist ebenfalls ziemlich interessant: https://corecursive.com/066-sqlite-with-richard-hipp/#testin...

    DO-178B. Das ist ein Qualitätsstandard für sicherheitskritische Luftfahrtprodukte ... Die Tests müssen dafür sorgen, dass jede Verzweigungsoperation im resultierenden Binärcode mindestens einmal ausgeführt und mindestens einmal durchlaufen wird ... Das dauerte ein Jahr mit 60-Stunden-Wochen ... Es hat einen enormen Unterschied gemacht. In den 8 oder 9 Jahren danach gab es praktisch keine Bugs.

  • Dieser Teil erinnert mich an Exhaustiveness Checking in TypeScript-Code. Ich versuche, es immer zu verwenden.
    https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

    • Das neuere satisfies never eignet sich dafür sehr gut. Es ist auch praktisch, wenn man aus Geschmacksgründen if else-Ketten verwendet.

    • ts-pattern könnte dir gefallen.

      https://github.com/gvergnaud/ts-pattern

  • Wenn man nur die Fälle betrachtet, in denen jedem nicht völlig trivialen if ein explizites else hinzugefügt wird, frage ich mich, wie viel einfacher dieser Code geworden wäre, wenn die Kubernetes-Autoren ihn um strukturelles Pattern Matching statt um if/else-Blöcke herum entworfen hätten.
    Mehrere Mainstream-Sprachen mit Unterstützung für strukturelles Pattern Matching bieten Werkzeuge, die zur Compile-Zeit prüfen, ob ein Match vollständig ist; schon das allein könnte eine idiomatische Lösung sein, die zugleich die Informationsdichte des Codes erhöht.

  • Diskussion von 2018: https://news.ycombinator.com/item?id=18772873

  • Ich habe den Code nur grob überflogen, aber ehrlich gesagt sieht er nicht so schlecht aus. Es gibt Dinge, die ich anders gemacht hätte, aber ich habe schon deutlich schlimmeren Code gesehen.
    Zumindest folgt dieser Code einer Regel, alles wirkt durchdacht, und man bekommt den Eindruck, dass in diesem Chaos eine Methode steckt. Ich würde solchen Code jederzeit dem typischen Mischmasch vorziehen, das man oft sieht: zusammengewürfelte Stile, faules Coden und unlogische Struktur.

  • Ich frage mich, warum man neue „Sicherheits“-Praktiken erfindet und dabei dokumentierte Best Practices der Softwaretechnik ignoriert.
    Module mit 2.000 Zeilen, Methoden mit 200 Zeilen und drei- bis vierstufig verschachtelte ifs gelten als schädlich. Kommentare, die nur sagen, was etwas tut statt warum, sind ebenfalls nicht hilfreich und geraten leicht in Widerspruch zum tatsächlichen Code. Auch unnötige Verwendung von nil ist zu sehen. Selbst ohne auf tiefere Themen wie Kopplung oder das Single-Responsibility-Prinzip einzugehen, fallen diese Punkte an der Oberfläche auf.

    • Wenn du glaubst, dass solche Dinge schädlich sind, empfehle ich „John Carmack on Inlined Code“ zu lesen.
      http://number-none.com/blow/john_carmack_on_inlined_code.htm...

      „Der Flugsteuerungscode der Armadillo-Rakete umfasst nur ein paar Tausend Zeilen, also nahm ich die Haupt-tic-Funktion und begann, alle Unterroutinen zu inlinen. Ich kann nicht sagen, dass ich einen versteckten Bug gefunden hätte, der einen echten Absturz hätte verursachen können, aber ich fand einige Variablen, die mehrfach gesetzt wurden, und ein paar Kontrollflüsse, die etwas verdächtig aussahen, und der endgültige Code wurde kleiner und sauberer.“

      Wenn Carmack in diesem Ansatz Wert gesehen hat, sollte man ihn wohl nicht vorschnell abtun. Auch die nachfolgenden Kommentare sind lesenswert.

      „In den Jahren seit dem Schreiben dieses Artikels bin ich selbst in C/C++ deutlich positiver gegenüber rein funktionaler Programmierung geworden, solange sie in vernünftigen Grenzen bleibt ... Wenn es zu schwer handhabbar wird, finde einen Weg, den Block in eine reine Funktion auszulagern.“

    • Manchmal gibt es eben „keinen anderen Weg™“.
      Willkürliche Zeilenlimits führen leicht zu unnötiger Fragmentierung. Rechnet man Includes, Lizenzen, Glue-Code und Kommentare dazu, entsteht schwer zugänglicher Spaghetticode. Wenn man versucht, Methoden in Hochleistungscode auf 200 Zeilen zu beschränken, kann die Performance abstürzen wie Ikarus im Flug.

Wenn man die Kommentare im Code liest, sieht man, dass hier enorm viel Know-how hineingeflossen ist, um diesen Code auf ein einzelnes Modul zu vereinfachen und ihn zugänglich sowie – noch wichtiger – nachhaltig wartbar zu machen. Für jemanden, der die Sprache oder die Logik nicht kennt, sind Kommentare, die einen Überblick darüber geben, was der Code tut, sehr nützlich. Nach sechs Monaten wirkt selbst der eigene Code fremd, also sind sie auch für einen selbst hilfreich.

Kommentare sind Teil des Codes und der Codebase. Wenn man beim Ändern des umgebenden Codes die Kommentare nicht mit aktualisiert, bringt man Dokumentations-Bugs in den Code ein. Nur weil der Compiler sie nicht verarbeitet, heißt das nicht, dass sie kein funktionaler Bestandteil sind. Im Kern sind Kommentare Wissen und in den Code eingebettete Forschungsnotizen; bei der Wartung des geschriebenen Codes können sie wertvoller sein als der ausgeführte Code.

Best Practices sind keine Gesetze oder strengen Regeln, sondern Richtlinien. Man sollte sie anwenden, wenn sie zur Codebase passen, und nicht blind befolgen und dadurch eine problematische Codebase schaffen. Manchmal muss man Regeln biegen und eigene Regeln aufstellen; wenn man weiß, was man tut, ist das völlig akzeptabel.

  • Ich habe ziemlich lange auf diese „sichere“ Weise geschrieben, aber damit deutlich mehr Bugs erzeugt als mit Railway-orientierter Fehlerbehandlung durch Early Returns, und das Beheben dauerte auch viel länger.
    Wenn man jedem if-Block ein explizites else anhängt, explodiert die Komplexität, weil man den aktuellen Kontext im Kopf behalten muss. Ich halte es für sinnvoll, diese Regel in „Jeder if-Bedingungsblock gibt frühzeitig zurück oder hat einen zugehörigen else-Block“ zu ändern. Das Muster if (cond) { Sonderbehandlung } ist definitiv deutlich gefährlicher als Early Returns und erschwert das Schlussfolgern.

  • Es gibt nicht das eine offizielle Bündel von Best Practices.
    Weder die Länge einer Funktion noch die Anzahl der Codezeilen in einer Datei ist an sich schädlich oder nützlich. Jede Sprache hat ihre Sicht darauf, wie Code organisiert werden sollte, aber nichts davon kann als „beste Praxis“ beansprucht werden. Go ist keine Sprache, die es bevorzugt, Code in viele kleine Dateien aufzuteilen.

  • Eine 200 Zeilen lange Methode ist nicht grundsätzlich falsch. Wenn der interne Code linear ist und auf derselben Abstraktionsebene bleibt, kann das die beste Wahl sein.
    Die Alternative, 40 Methoden mit je 5 Zeilen zu erstellen, kann schlechter sein. Um das Ganze zu verstehen, muss man überall hin- und herspringen, und man kann auch die Aufrufreihenfolge durcheinanderbringen. Es gibt 40! mögliche Permutationen.

  • Solcher Code wirkt wie ein idealer Kandidat, um ihn in ein deklaratives, regelbasiertes, tabellengesteuertes System zu überführen.
    So ein Ansatz ist leichter zu verstehen und zu verifizieren als ein ad hoc zusammengewürfelter imperativer Code voller if-Klauseln. Unordentlicher Code dieser Art ist normalerweise ein Hinweis darauf, dass eine Abstraktion fehlt.

    • Die Philosophie von Go läuft im Grunde darauf hinaus, alles ziemlich direkt so herunterzuschreiben, als würde man Code, den man sonst in C geschrieben hätte, übertragen, und gar nicht erst zu versuchen, etwas zu abstrahieren.