อย่าทำให้โค้ดนี้เรียบง่ายขึ้น

 (github.com/kubernetes)
2 คะแนน โดย GN⁺ 2024-08-07 | 1 ความคิดเห็น | แชร์ทาง WhatsApp
  • pv_controller.go ของ Kubernetes เป็นคอนโทรลเลอร์ที่ซิงโครไนซ์ การ bind ระหว่าง PV/PVC และตั้งแต่คอมเมนต์ต้นไฟล์ก็ระบุชัดว่า “อย่าทำให้เรียบง่ายขึ้น ให้คงสไตล์ space shuttle ไว้”
  • สไตล์นี้คือการมี else คู่กับ if ทุกตัว และใส่คอมเมนต์ไว้แม้กับเงื่อนไขที่ดูชัดเจน เพื่อแสดง สาขาที่ผ่านการพิจารณาแล้วและเจตนา ไว้ในโค้ด
  • แกนของการออกแบบคือ pointer สองทาง ระหว่าง pvc.Spec.VolumeName และ pv.Spec.ClaimRef ซึ่งช่วยให้รับมือกับการแข่งขัน การลบ การแก้ไขโดยผู้ใช้ และการ bind พร้อมกันได้แบบกู้คืนได้ในสภาพแวดล้อมที่ไม่มี transaction
  • คอนโทรลเลอร์ผูกการเฝ้าดูการเปลี่ยนแปลงของ PV/PVC, cache ภายใน, worker queue เดี่ยว, การบันทึก event, dynamic provisioning และอินเทอร์เฟซ CSI migration เข้าด้วยกัน เพื่อจัดการ การเปลี่ยนสถานะของการ bind
  • สาขาเงื่อนไขและคอมเมนต์ที่ยืดยาวเป็นกลไกสำหรับรักษาความรู้เชิงธุรกิจของพฤติกรรมและบริบทการกู้คืนจากความล้มเหลว ดังนั้นการเปลี่ยนแปลงในอนาคตก็ควรทำตามสไตล์เดียวกัน

บทบาทและหลักการเขียนของ pv_controller.go

  • pv_controller.go เป็นไฟล์ implementation ของ PersistentVolumeController ในแพ็กเกจ persistentvolume ของ Kubernetes
  • คอนโทรลเลอร์นี้ทำให้สถานะของ PersistentVolumeClaim และ PersistentVolume สอดคล้องกัน
    • cache controller ที่เฝ้าดูการเปลี่ยนแปลงของ PersistentVolume
    • cache controller ที่เฝ้าดูการเปลี่ยนแปลงของ PersistentVolumeClaim
    • ซิงโครไนซ์สถานะ PV/PVC จาก event การเปลี่ยนแปลงของอ็อบเจ็กต์ทั้งสอง
  • คอมเมนต์ต้นไฟล์เตือนซ้ำ ๆ ว่า อย่าทำให้โค้ดนี้เรียบง่ายขึ้น
    • ชื่อสไตล์คือ space shuttle style
    • เป็นแนวทางที่มี else คู่กับคำสั่ง if ทุกตัว
    • เป้าหมายคือระบุทุกสาขาให้ชัดเจน ยกเว้นการเช็ก error แบบง่าย ๆ
    • แม้พฤติกรรมที่ดูชัดเจนก็เขียนเป็นคอมเมนต์ เพื่อให้ผู้ดูแลรักษาตามความซับซ้อนของการ bind ได้

เหตุผลที่ต้องคง space shuttle style

  • เดิมทีคอนโทรลเลอร์นี้เป็นผลจากการรวมงานที่เคยแยกอยู่ในคอนโทรลเลอร์สามตัวให้มาอยู่ตัวเดียว
  • ในกระบวนการทำให้ subsystem ของ PV เรียบง่ายขึ้น จำเป็นต้องมีวิธีที่จัดการทุกเงื่อนไขอย่างชัดเจนในโค้ด
  • ผลลัพธ์คือโค้ดอาจดูยืดยาวและมีคอมเมนต์กับสาขาเงื่อนไขจำนวนมาก
  • ความยืดยาวนี้เป็นกลไกสำหรับทิ้ง ความรู้เชิงธุรกิจและบริบทของพฤติกรรมการ bind ไว้ในโค้ด
  • เมื่อแก้ไฟล์นี้ต้องรักษา space shuttle style ไว้ และหากจำเป็นให้เพิ่มสาขาและคอมเมนต์ด้วยแนวทางเดียวกัน

การออกแบบหลัก: pointer สองทางของ PV และ PVC

  • แกนของการออกแบบคือ pointer สองทาง ระหว่าง PV กับ PVC
    • pointer ฝั่ง PVC: pvc.Spec.VolumeName
    • pointer ฝั่ง PV: pv.Spec.ClaimRef
  • ความเป็นสองทางนี้จัดการได้ยากในระบบที่ไม่มี transaction แต่จำเป็นเพื่อรับประกันการทำงานที่ถูกต้องแม้เกิดความขัดข้อง
  • หาก rogue HA controller instance สร้าง race condition อาจเกิดการ bind หลายรายการที่แยกไม่ออก และทำให้มี ความเป็นไปได้ที่ข้อมูลจะสูญหาย
  • โดยพื้นฐานแล้วคอนโทรลเลอร์ออกแบบให้ทำงานในโหมด high availability แบบ active-passive
    • การเปลี่ยนผ่านของอ็อบเจ็กต์ถูกออกแบบให้ทำงานได้แม้ใน active-active HA
    • อย่างไรก็ตาม หากคอนโทรลเลอร์ active สองตัวชนกันบ่อย ประสิทธิภาพอาจลดลงได้

วิธี bind และเงื่อนไขการกู้คืน

  • คอนโทรลเลอร์รองรับ อ็อบเจ็กต์ pre-bound แบบสองทาง
    • PVC ที่ต้องการ PV เฉพาะ
    • PV ที่ถูกจองไว้สำหรับ PVC เฉพาะ
  • การ bind ดำเนินเป็นสองขั้นตอน
    • แก้ไข PV.Spec.ClaimRef ก่อน
    • จากนั้นแก้ไข PVC.Spec.VolumeName
  • ในทุกจุดของกระบวนการนี้ PV หรือ PVC อาจถูกผู้ใช้หรือคอนโทรลเลอร์อื่นแก้ไขหรือลบได้
  • คอนโทรลเลอร์ตั้งแต่สองตัวขึ้นไปอาจพยายาม bind volume และ claim ต่างชุดพร้อมกันก็ได้
  • คอนโทรลเลอร์ต้องสามารถ กู้คืน จากสถานการณ์ขัดแย้งเหล่านี้ได้

องค์ประกอบหลักของ struct คอนโทรลเลอร์

  • PersistentVolumeController มี lister, ฟังก์ชัน sync ของ informer, Kubernetes client, event recorder, volume plugin manager และส่วนอื่น ๆ ที่จำเป็นต่อการซิงโครไนซ์ PV/PVC
  • เวอร์ชันล่าสุดที่ทราบของ PV/PVC ถูกเก็บใน cache ภายใน
    • volumes persistentVolumeOrderedIndex
    • claims cache.Store
  • cache นี้สะท้อนทั้งเวอร์ชันล่าสุดที่บันทึกไว้ใน API server และเวอร์ชันที่เข้ามาผ่าน event ของ etcd
  • การ bind หนึ่งครั้งอาจสร้าง event ได้ประมาณสี่รายการ
    • อัปเดต volume.Spec
    • อัปเดต volume.Status
    • อัปเดต claim.Spec
    • อัปเดต claim.Status
  • หากไม่มี cache ภายใน เมื่อ informer ถือสถานะเก่าอยู่ อาจพยายามแก้การ bind ที่เสร็จแล้วซ้ำ
  • เมื่อพยายามเขียนกลับไปยัง API server ในจังหวะนั้น อาจเกิด version conflict กับอ็อบเจ็กต์ที่บันทึกไว้แล้ว

Work queue และข้อจำกัดด้าน concurrency

  • คอนโทรลเลอร์มี workqueue แยกสำหรับประมวลผล claim และ volume
    • claimQueue
    • volumeQueue
  • แต่ละ queue ต้องมี worker thread เพียงหนึ่งตัวเท่านั้น
  • โดยเฉพาะ syncClaim() ไม่สามารถ re-enter ได้
  • หาก syncClaim() สองตัวทำงานพร้อมกัน อาจเกิดปัญหาต่อไปนี้
    • bind claim สองรายการต่างกันเข้ากับ volume เดียวกัน
    • bind claim หนึ่งรายการเข้ากับ volume สองตัว
  • คอนโทรลเลอร์สามารถกู้คืนสถานการณ์เหล่านี้ได้ด้วย version error จาก API server และการตรวจสอบของตัวเอง แต่แนวทาง multi-worker อาจทำให้ความเร็วโดยรวมลดลง

syncClaim: จุดเข้าของการซิงโครไนซ์ PVC

  • syncClaim เป็นเมธอดหลักที่ถูกเรียกเมื่อ claim ถูกสร้าง อัปเดต หรือซิงโครไนซ์ตามรอบเวลา
  • เมธอดนี้ไม่แยกประเภท event
  • ขั้นแรกจะตั้ง migration annotation ที่ถูกต้องให้ PVC และอัปเดตไปยัง API server หากจำเป็น
  • จากนั้นจะแยกสาขาตามว่ามี annotation AnnBindCompleted หรือไม่
    • หากไม่มี annotation จะไปที่ syncUnboundClaim
    • หากมี annotation จะไปที่ syncBoundClaim
  • การประมวลผลจริงถูกแบ่งเป็นเมธอดสำหรับ unbound claim และ bound claim เพื่อให้อ่านง่าย

checkVolumeSatisfyClaim: ตรวจสอบข้อกำหนดของ PV

  • checkVolumeSatisfyClaim ตรวจสอบว่า PV ที่ถูกร้องขอตอบสนองข้อกำหนดของ PVC หรือไม่
  • เงื่อนไขการตรวจสอบถูกระบุไว้ในโค้ดอย่างชัดเจน
    • หาก PV มี DeletionTimestamp ให้ถือว่า error
    • หากความจุของ PV น้อยกว่าความจุที่ PVC ขอ ให้ถือว่า error
    • หาก storageClassName ต่างกัน ให้ถือว่า error
    • หาก feature gate VolumeAttributesClass เปิดอยู่ จะตรวจว่าค่า VolumeAttributesClassName ตรงกันหรือไม่
    • หาก feature gate ปิดอยู่แต่ claim หรือ volume มี VolumeAttributesClassName ให้ถือว่า error
    • หาก volumeMode ไม่เข้ากัน ให้ถือว่า error
    • หาก access mode ไม่เข้ากัน ให้ถือว่า error
  • หากผ่านทุกเงื่อนไข จะคืนค่า nil

การจัดการ event ของ PVC แบบ delayed binding

  • emitEventForUnboundDelayBindingClaim สร้าง event เพื่อให้ข้อมูลแก่ claim ที่ยังไม่ถูก bind ในโหมด delayed binding
  • reason เริ่มต้นคือ WaitForFirstConsumer
  • ข้อความเริ่มต้นระบุว่าจะรอการ bind จนกว่า consumer ตัวแรกจะถูกสร้างขึ้น
  • หากมี Pod ที่อ้างอิง PVC นี้และยังไม่ได้ถูก schedule อยู่ reason จะเปลี่ยนเป็น WaitForPodScheduled
    • หากมีหลาย Pod จะใส่ชื่อ Pod ทั้งหมดในข้อความ
    • ใน volume scheduling จะพิจารณา Pod เพียงตัวเดียว แต่เพราะไม่รู้ว่าจะใช้ Pod ใด จึงใส่ทั้งหมด

syncUnboundClaim: การจัดการ PVC ที่ยังไม่ถูก bind

  • หาก claim.Spec.VolumeName ว่าง แปลว่าผู้ใช้ยังไม่ได้ร้องขอ PV เฉพาะ
  • ในกรณีนี้คอนโทรลเลอร์จะตรวจสอบโหมด delayed binding ของ claim และหา PV ที่เหมาะสมที่สุดด้วย findBestMatchForClaim
  • หากไม่มี PV ที่เหมาะสม จะประมวลผลตามลำดับต่อไปนี้
    • หากกำหนด StorageClass เริ่มต้นได้ ให้อัปเดต PVC แล้วจบการซิงโครไนซ์
    • หากเป็น delayed binding และยังไม่อยู่ในสถานะ provisioning ให้สร้าง event สำหรับการรอ
    • หาก claim มี StorageClass ให้พยายามทำ dynamic provisioning ด้วย provisionClaim
    • มิฉะนั้น ให้บันทึก event FailedBinding ว่าไม่มี PV ที่ใช้ได้และไม่มี StorageClass
  • หากมี PV ที่เหมาะสม จะเรียก bind เพื่อ bind PV กับ PVC
    • เมื่อสำเร็จ จะบันทึก metric ของงาน provision + binding และล้าง timestamp cache
    • หากเกิด error ระหว่างการบันทึก syncClaim รอบถัดไปจะทำให้การ bind เสร็จสมบูรณ์

การจัดการ PVC ที่ร้องขอ PV เฉพาะ

  • หาก claim.Spec.VolumeName ไม่ว่าง แปลว่าผู้ใช้ร้องขอ PV เฉพาะ
  • หาก PV ที่ร้องขอไม่มีใน cache จะอัปเดตสถานะ PVC เป็น Pending แล้วลองใหม่ภายหลัง
  • หากมี PV ที่ร้องขอและ volume.Spec.ClaimRef ไม่มีค่า แปลว่า PV ยังไม่ได้ถูก claim
    • ตรวจสอบข้อกำหนดด้วย checkVolumeSatisfyClaim
    • หากไม่ตรงข้อกำหนด ให้บันทึก event VolumeMismatch และคง PVC ไว้ที่ Pending
    • หากตรงข้อกำหนด ให้เรียก bind
  • หาก PV ที่ร้องขอถูก claim โดย PVC นี้อยู่แล้ว ให้เรียก bind เพื่อทำให้การ bind เสร็จสมบูรณ์
  • หาก PV ที่ร้องขอถูกผูกกับ claim อื่นแล้ว จะจัดการดังนี้
    • หาก claim ไม่มี annotation ที่บอกว่าถูก bind โดย controller ให้บันทึก event FailedBinding และคงไว้ที่ Pending
    • หากดูเหมือนว่า controller เป็นผู้ bind แต่กลับถูกผูกกับ claim อื่น ให้คืน error ในสถานะ “should never happen”

syncBoundClaim: การจัดการ PVC ที่ถูก bind แล้ว

  • syncBoundClaim จัดการ PVC ที่มี annotation AnnBindCompleted
  • หากเป็น claim ที่ถูก bind แล้วแต่ claim.Spec.VolumeName ว่าง จะเปลี่ยนสถานะ claim เป็น ClaimLost
    • ข้อความ event ระบุว่า bound claim สูญเสียการอ้างอิงถึง PV และข้อมูลใน volume สูญหายแล้ว
  • หาก PV ที่ claim ชี้ไปไม่มีอยู่ ก็จะเปลี่ยนเป็น ClaimLost เช่นกัน
    • ข้อความ event ระบุว่า bound claim สูญเสีย PersistentVolume และข้อมูลสูญหายแล้ว
  • หากมี PV อยู่แต่ volume.Spec.ClaimRef ไม่มีค่า จะถือว่า volume กลับเป็น unbound แล้วเรียก bind อีกครั้ง
  • หาก ClaimRef.UID ของ PV เท่ากับ UID ของ claim จะถือว่าเป็นสถานะการ bind ปกติและเรียก bind
    • ในกรณีส่วนใหญ่ การเรียกนี้จะไม่ทำอะไร
  • หาก PV ชี้ไปยัง claimant อื่น จะตั้ง claim phase เป็นสถานะ terminal Lost

syncVolume: จุดเข้าของการซิงโครไนซ์ PV

  • syncVolume เป็นเมธอดหลักที่ถูกเรียกเมื่อ volume ถูกสร้าง อัปเดต หรือซิงโครไนซ์ตามรอบเวลา
  • ไม่แยกประเภท event
  • ขั้นแรกจะตั้ง migration annotation และ finalizer ที่ถูกต้องให้ PV และอัปเดตไปยัง API server หากจำเป็น
  • หาก volume.Spec.ClaimRef ไม่มีค่า จะถือว่าเป็น volume ที่ไม่ได้ใช้งานและตั้ง phase เป็น Available
  • หากมี ClaimRef แต่ UID ว่าง จะถือว่าเป็น PV ที่ถูกจองไว้สำหรับ PVC เฉพาะและตั้ง phase เป็น Available
    • PVC ดังกล่าวยังไม่ได้ bind กับ PV นี้ และ PVC sync จะเป็นผู้จัดการ

การจัดการ PV ที่หา claim ไม่พบ

  • หาก PV ถูก bind กับ claim คอนโทรลเลอร์จะหา PVC ด้วย namespace/name ใน ClaimRef
  • หากหา PVC ใน cache ไม่พบ จะตรวจสอบเพิ่มเติมในบางเงื่อนไข
    • ตรวจสอบอีกครั้งจาก informer cache
    • ตรวจสอบอีกครั้งจาก API server
  • สำหรับ PV ที่สร้างโดย external PV provisioner หรือ external PV binder ภายใต้โหลดสูง PVC อาจยังไม่ได้ซิงโครไนซ์เข้ามาใน cache ภายในเครื่อง
  • เพื่อไม่ให้ reclaim PVC ผิดพลาด จึงทำ การตรวจสอบซ้ำสองชั้น
  • หากตัดสินว่าไม่มี claim จะเปลี่ยน volume phase เป็น Released และเรียก reclaimVolume
    • หาก phase เดิมเป็น Failed จะไม่เขียนทับ
    • หาก reclaim policy เป็น Retain จะบันทึก log ว่า PV อ้างถึง claim ที่ไม่มีอยู่

กรณีที่การเชื่อมโยงระหว่าง PV และ PVC ไม่ตรงกัน

  • หากมี claim อยู่แต่ claim.Spec.VolumeName ว่าง แปลว่า PVC ยังไม่มีชื่อ PV
  • หาก volumeMode ไม่ตรงกัน จะบันทึก event VolumeMismatch ทั้งฝั่ง PV และ PVC แล้วข้าม syncClaim
  • หากไม่ใช่ mismatch จะเพิ่ม claim เข้า claimQueue เพื่อให้ syncClaim ถูกเรียกในไม่ช้า
    • วิธีนี้ช่วยให้การ bind volume ที่ถูก provision แล้วเร็วขึ้น
  • หาก Spec.VolumeName ของ claim เท่ากับชื่อ volume ปัจจุบัน จะถือว่าเป็นการ bind ปกติและอัปเดต phase ของ volume เป็น Bound
  • หาก claim ถูก bind กับ volume อื่นแล้ว จะจัดการตามสถานการณ์
    • หากเป็น volume ที่ถูก provision แบบ dynamic และ reclaim policy เป็น Delete จะทำเครื่องหมายเป็น Released แล้วเรียก reclaimVolume
    • หากเป็น volume ที่ controller bind ให้ จะเก็บกวาดด้วย unbindVolume
    • หากเป็น pointer ที่ผู้ใช้สร้างไว้ จะปล่อยไว้ตามเดิมแต่เรียก unbindVolume เพื่ออัปเดต phase และล้าง ClaimRef.UID

การอัปเดตสถานะและการส่ง event

  • updateClaimStatus บันทึก status ของ PVC ไปยัง API server
    • การเปลี่ยน phase
    • รีเซ็ต AccessModes, Capacity, CurrentVolumeAttributesClassName เมื่อไม่มี volume
    • อัปเดต access mode, capacity และชื่อ current volume attributes class เมื่อมี volume
  • มีเงื่อนไขที่อัปเดต capacity เฉพาะตอนที่ claim กลายเป็น Bound เท่านั้น
    • เพราะความแตกต่างระหว่างขนาด filesystem ของ PVC กับขนาด block device ของ PV อาจตั้งใจให้เป็นเช่นนั้น จึงไม่เขียนทับ capacity ของ claim ที่ bound อยู่แล้ว
  • หาก feature gate VolumeAttributesClass เปิดอยู่ จะตั้ง CurrentVolumeAttributesClassName ระหว่างที่เปลี่ยนจาก pending เป็น bound
    • หลังจากนั้นควรให้ resizer หรือ admin override จัดการ และหากคอนโทรลเลอร์ตั้งค่าต่อเนื่องอาจเกิด race condition ได้
  • updateClaimStatusWithEvent และ updateVolumePhaseWithEvent จะส่ง event เฉพาะเมื่อ status/phase เปลี่ยนจริงเท่านั้น

การกำหนด StorageClass เริ่มต้น

  • assignDefaultStorageClass หา StorageClass เริ่มต้นและกำหนดให้ claim เมื่อ claim ไม่มี storage class
  • claim ที่มี storage class อยู่แล้วจะถูกข้าม
  • หากไม่มี class เริ่มต้น จะไม่อัปเดตและคืนค่า false
  • หากมี class เริ่มต้น จะตั้งชื่อ class ใน claim.Spec.StorageClassName แล้วอัปเดตไปยัง API server

ขอบเขตของไฟล์และข้อจำกัดที่ระบุชัด

  • ตาม metadata ของไฟล์ที่แสดงบนหน้า GitHub pv_controller.go มี 2038 บรรทัด, 1864 LOC, 91 KB
  • เนื้อหาที่ให้มาครอบคลุมตั้งแต่ส่วนต้นของไฟล์ไปจนถึงจุดเริ่มต้นของฟังก์ชัน bindVolumeToClaim เท่านั้น ส่วนที่เหลือต่อไปยังลิงก์ raw view
  • ดังนั้นสรุปนี้จำกัดอยู่ที่โครงสร้างคอนโทรลเลอร์ คอมเมนต์ด้านการออกแบบ สาขาการซิงโครไนซ์หลัก และลอจิกอัปเดตสถานะที่ปรากฏในเนื้อหาโค้ดที่ให้มา

บทความที่เกี่ยวข้อง

1 ความคิดเห็น

 
GN⁺ 2024-08-07
ความคิดเห็นใน Hacker News

  • ผมไม่แน่ใจว่าแปลกไหมที่โค้ดในไฟล์นี้ให้ความรู้สึกเหมือน โค้ด Go ธรรมดาๆ จริงๆ เพราะเป็น Go เลยดูเยิ่นเย้อ และเพราะไม่ได้พึ่งพา abstraction ลึกๆ เลยดูยาวขึ้น แต่ตัวโค้ดเองก็ดูเป็นแบบทั่วไป
    abstraction เป็นดาบสองคม วิธีแบบนี้ก็โอเค และถ้าไม่มีคำนำ ผมคงไม่คิดทบทวนเรื่องสไตล์การเขียนซ้ำด้วยซ้ำ อาจเป็นความแตกต่างที่เกิดจากการมีประสบการณ์กับซอฟต์แวร์องค์กร มากกว่าซอฟต์แวร์ระบบ สำหรับคนที่มีส่วนร่วมกับ Kubernetes อย่างสม่ำเสมอ คอมเมนต์เหล่านี้อาจดูไม่จำเป็น แต่ถ้าเป็นโค้ดที่ผู้อ่านในอนาคตไกลๆ ในสภาพแวดล้อมองค์กรต้องอ่านโดยไม่มีบริบท ที่ระดับความซับซ้อนนี้ผมอาจใส่คอมเมนต์มากกว่านี้ด้วยซ้ำ

    • เมื่อก่อนโค้ดแบบนี้รู้สึกเป็นเรื่องปกติ แต่ในช่วงประมาณ 10 ปี ที่ผ่านมา ดูเหมือนหลายคนให้ค่ากับความสั้นมากกว่าความชัดเจน
      โดยเฉพาะในโค้ดสำคัญแบบนี้ ผมชอบความชัดเจนมากกว่าอย่างมาก ในอาชีพผมเคยเจอหลายครั้งที่โค้ดรวมเงื่อนไขหลายอย่างเข้าด้วยกันและละคอมเมนต์ที่อธิบายบริบททางธุรกิจกับความหมาย ทำให้ตัดสินไม่ได้ว่าพฤติกรรมปัจจุบันเป็นสิ่งที่ตั้งใจไว้หรือเป็นเรื่องบังเอิญ วิธีแบบนี้มักไม่ใช่โค้ดที่ทนต่อการเปลี่ยนแปลง แต่กลายเป็นโค้ดที่ขัดขวางการเปลี่ยนแปลง และอย่างน้อยก็ทำให้คนที่ไม่ใช่ผู้เขียนแก้ไขได้ยาก การสร้าง รั้วของเชสเตอร์ตัน ที่ไม่จำเป็นนั้นขัดกับความสามารถในการบำรุงรักษา

    • คอมเมนต์นี้น่าจะถูกเพิ่มเข้ามาหลังจากมีคนพยายามทำให้โค้ดง่ายขึ้นแล้วล้มเหลว เพื่อเตือนผู้ดูแลในอนาคตให้คิดอีกครั้งก่อนจะลองทำแบบเดียวกัน
      คอมมิตที่เพิ่มคำเตือนคือ "Add note about space-shuttle code style"[1] และคอมมิตก่อนหน้านั้นทันทีคือ "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...

    • ผมก็เคยคิดคล้ายๆ กัน แต่พอเห็น if ที่ซ้อนกันลึกๆ ก็เปลี่ยนความคิด ตรงส่วนนั้นผมน่าจะทำเป็นแขนงแบบ return ก่อน แน่นอน
      ให้ความรู้สึกเหมือนทำเสร็จแค่ขั้นแรกของ "ทำให้ใช้ได้ ทำให้เร็ว ทำให้สวย" แล้วไม่ได้ทำขั้น "ทำให้สวย" เวลาแก้ปฏิสัมพันธ์ของสถานะที่ยุ่งยาก ผมก็เคยเขียนโค้ดหน้าตาไม่สวยและคอมเมนต์เยอะแบบนี้ แต่ปกติจะจัดระเบียบสักหน่อยก่อนรีวิว บางทีอาจดีกว่าถ้าติดแบนเนอร์ใหญ่ไว้บนสุดของไฟล์ว่า "อย่าพยายามทำให้โค้ดนี้ง่ายขึ้น" แต่ถึงอย่างนั้น มันก็ไม่ได้แย่มากแน่นอน

    • แม้มันอาจดูแปลก แต่คุณไม่ได้คิดอยู่คนเดียว ผมเองก็มองว่าโค้ดนี้ปกติอย่างสมบูรณ์ ผมเคยเขียนโค้ดและคอมเมนต์แบบนี้ในคอมโพเนนต์ที่รู้สึกว่าสำคัญต่อความน่าเชื่อถือของระบบ
      ผมไม่เคยเห็นด้วยกับกระแส "โค้ดไร้คอมเมนต์" และเวลาย้อนกลับมาดูหลังผ่านไปหลายเดือนหรือหลายปี คอมเมนต์ที่ผมเขียนไว้ก็มักมีค่ามากสำหรับตัวผมในอนาคตบ่อยครั้ง นึกภาพไม่ออกเลยว่าจะไล่ประกอบตรรกะที่ฝังอยู่ในคอมโพเนนต์ที่ซับซ้อนระดับนี้ขึ้นมาใหม่โดยไม่มีคอมเมนต์ที่แข็งแรงได้อย่างไร

    • โดยเฉพาะคำอธิบายที่บอกว่า if ทุกอันมีคอมเมนต์ else ที่คู่กัน ดูเหมือนไม่ได้เป็นจริงอย่างมั่นคงนัก if จำนวนมากที่ไม่มีคู่เป็นแค่การตรวจ if (err != nil) { หรือการ return ก่อน แบบอื่นๆ แต่ถึงตัดพวกนั้นออกไป ก็ยังดูเหมือนมี if ที่ไม่มีคู่กันอยู่
      อย่างไรก็ตาม จากประสบการณ์ซอฟต์แวร์องค์กร คอมเมนต์เพิ่มเติมก็ไม่ได้มีเยอะเสมอไป ใน codebase มีคอมเมนต์ // end if ระบาดเหมือนโรค แต่คอมเมนต์ที่อธิบายจริงๆ กลับพบได้น้อย

  • บทความเกี่ยวกับคุณภาพซอฟต์แวร์ของ Space Shuttle: https://archive.is/HX7n4
    ถ้าตัดตอนมา เหตุผลที่ซอฟต์แวร์นี้น่าทึ่งไม่ใช่ว่ามันทำงานได้มากแค่ไหน แต่อยู่ที่มันทำงานได้ดีเพียงใด กล่าวคือ ไม่เคยแครช ไม่ต้องรีบูต ไม่มีบั๊ก และใกล้เคียงความสมบูรณ์แบบในระดับที่มนุษย์ทำได้ เวอร์ชันสามรุ่นสุดท้ายมีรุ่นละ 420,000 บรรทัด แต่มีข้อผิดพลาดเพียงรุ่นละหนึ่งรายการ และทั้ง 11 เวอร์ชันสุดท้ายมีข้อผิดพลาดรวม 17 รายการ ว่ากันว่าโปรแกรมเชิงพาณิชย์ที่มีความซับซ้อนเท่ากันน่าจะมีข้อผิดพลาดประมาณ 5,000 รายการ

    • ผมสงสัยว่า "เวอร์ชันสามรุ่นสุดท้ายมีรุ่นละ 420,000 บรรทัด และมีข้อผิดพลาดรุ่นละหนึ่งรายการ" หมายความว่าอะไรกันแน่ ถ้าแต่ละรุ่นในสามรุ่นนั้นมีบั๊กพอดีหนึ่งตัว นั่นเป็นการพูดแบบแปลกๆ หรือเปล่าว่าการแก้สองครั้งก่อนหน้าไม่ทำงาน หรือใส่บั๊กใหม่เข้าไป?
    • น่าจะน่าสนใจถ้าเปรียบเทียบว่าวิธีของ NASA กับวิธีของ SpaceX ต่างกันอย่างไร SpaceX ก็ทำภารกิจที่มีมนุษย์โดยสารแล้ว ดังนั้นข้อกำหนดน่าจะค่อนข้างคล้ายกัน
    • 5000 / 17 ≈ 295 ถ้าจะสมมติว่าโปรแกรมเชิงพาณิชย์ที่มีความซับซ้อนเท่ากันใช้ ชั่วโมงแรงงานคนน้อยกว่า 295 เท่า จะถือว่ายุติธรรมหรือไม่?
    • ปัญหาของระเบียบวิธีพัฒนา Space Shuttle คือมันแพงและช้ามหาศาล แต่ก็ยังไม่ใช่ ไร้บั๊ก 100%
      มันแพงและช้ามากจนการพิสูจน์ความถูกต้องของซอฟต์แวร์ด้วย proof assistant สมัยใหม่ น่าจะถูกกว่า เร็วกว่า และปลอดภัยกว่าจริงๆ มาก โครงการอย่าง seL4, CompCert แสดงให้เห็นว่าควรทำอย่างไร
    • นี่เป็นหนึ่งในบทความที่ผมชอบที่สุด น่าทึ่งที่บทความบนอินเทอร์เน็ตจากปี 1996 ยังเข้าถึงได้อยู่

  • ผมเข้าใจเจตนาของ // KEEP THE SPACE SHUTTLE FLYING. แต่ก็น่าขำอยู่หน่อยที่ในคอมเมนต์อ้างถึงระบบที่ไม่ได้ใช้งานแล้วเพราะมีประวัติความปลอดภัยไม่ดี
    อีกสัก 10 ปี ผู้คนจะยังจดจำ Space Shuttle ในแง่ดีไหม?

    • ปัญหาความปลอดภัยของ Space Shuttle ส่วนใหญ่เป็น ปัญหาฮาร์ดแวร์ ไม่ใช่ปัญหาซอฟต์แวร์
      ใน "Appendix F - Personal Observations on Reliability of Shuttle" [0] ซึ่งเป็นภาคผนวกของ Richard Feynman ในรายงานอุบัติเหตุ Challenger ปี 1986 กล่าวไว้ดังนี้

      โดยสรุป ระบบตรวจสอบซอฟต์แวร์คอมพิวเตอร์และทัศนคติต่อมันมีคุณภาพสูงสุด ไม่เห็นกระบวนการค่อยๆ หลอกตัวเองโดยลดมาตรฐานลง ซึ่งเป็นลักษณะที่พบในระบบความปลอดภัยของ Solid Rocket Booster หรือ Space Shuttle Main Engine

      เขาเน้นเป็นพิเศษถึงคุณภาพของซอฟต์แวร์ avionics ในฐานะตัวอย่างว่าโครงการรัฐบาลขนาดใหญ่และซับซ้อนอย่าง Shuttle ก็สามารถทำวิศวกรรมได้อย่างถูกต้อง และไม่ได้ถูกกำหนดมาโดยตัวมันเองว่าจะต้องคุณภาพต่ำหรือเสี่ยงภัย

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

  • เคยพาคนและอุปกรณ์ขึ้นสู่อวกาศและพากลับบ้านในภารกิจที่ประสบความสำเร็จมากกว่า 100 ครั้งอย่างมาก ทุกวันนี้ก็ยังมองในแง่ดี และมีแนวโน้มว่าจะเป็นเช่นนั้นต่อไป ในแง่ความก้าวหน้าของมนุษย์และผลสุทธิ ถือว่าเป็นความสำเร็จ

  • สิ่งที่ทำให้ Shuttle ยุติลงไม่ใช่ประวัติความปลอดภัยที่ย่ำแย่ แต่เป็น ต้นทุน และการคาดการณ์ว่าความปลอดภัยในอนาคตจะลดลง
    แม้อุบัติเหตุ Shuttle สองครั้งจะทำให้นักบินอวกาศเสียชีวิตมากกว่าภัยพิบัติอื่น ๆ ของ NASA แต่เมื่อพิจารณาความยากของสิ่งที่เกิดขึ้นจริง ประวัติด้านความปลอดภัยก็น่าทึ่งมาก โค้ดดูดีมาก

  • สถานการณ์ของ Space Shuttle ซับซ้อนกว่าการบอกง่าย ๆ ว่าความปลอดภัยไม่ดี หากดูตามภารกิจ สถิติดีกว่ายานปล่อยอื่น ๆ ด้วยซ้ำ Shuttle มีภารกิจถึงขั้นเสียชีวิต 2 ครั้งจาก 135 ครั้ง ส่วน Soyuz สมัยสหภาพโซเวียตมี 2 ครั้งจาก 66 ครั้ง และ SpaceShipTwo มีสถิติแย่อย่างน่ากลัวคือภารกิจถึงขั้นเสียชีวิต 1 ครั้งจากการบินเพียง 12 ครั้ง
    อย่างไรก็ตาม Space Shuttle มีความจุลูกเรือมากกว่าที่ภารกิจส่วนใหญ่ต้องการอยู่มาก ต่างจาก Apollo หรือ Soyuz ที่มี 3 คน มันบรรทุกได้สูงสุด 8 คน และเมื่อคิดว่าภารกิจส่วนใหญ่ของสหภาพโซเวียต/Roscosmos, ESA, CNSA เป็นภารกิจอัตโนมัติไร้คนขับทั้งหมด ก็ไม่มีลูกเรือให้ตกอยู่ในความเสี่ยงตั้งแต่แรก บางทีอุปมานี้อาจเหมาะกับ Kubernetes มากกว่า: เป็นระบบที่ถูกวิศวกรรมมาอย่างสูง ทรงพลัง ใช้งานได้หลากหลาย แต่ต้องใส่ใจมาก และอาจถูกใช้มากเกินความจำเป็นเล็กน้อย

  • หากใช้มาตรวัดที่พบบ่อยที่สุดคือจำนวนผู้โดยสาร-ไมล์ Space Shuttle จัดอยู่ในกลุ่มยานพาหนะที่ปลอดภัยที่สุดเท่าที่เคยสร้างและบินมา
    ในฐานะคนที่วัยเด็กอยู่ในยุค 1980s พอดี ถามตรง ๆ ว่า จะไม่จดจำมันในแง่ดีได้อย่างไร หรือว่าเด็กเกินไปจนมองโปรแกรมนี้กับภารกิจและความสำเร็จทั้งหมดแบบย้อนหลังเท่านั้น และมีแต่มุมมองที่ถูกบรรยากาศยุคปัจจุบันซึ่งเน้นผู้รับเหมาด้านอวกาศเอกชนหล่อหลอมไว้?

  • เรื่องที่ Richard Hipp ปรับโค้ด SQLite ให้สอดคล้องกับมาตรฐานการบินก็น่าสนใจมากเช่นกัน: https://corecursive.com/066-sqlite-with-richard-hipp/#testin...

    DO-178B เป็นมาตรฐานคุณภาพสำหรับผลิตภัณฑ์การบินที่มีความสำคัญต่อความปลอดภัย... การทดสอบต้องทำให้แต่ละ branch operation ในโค้ดไบนารีผลลัพธ์ถูก execute อย่างน้อยหนึ่งครั้ง และผ่านอย่างน้อยหนึ่งครั้ง... ใช้เวลา 1 ปี ทำงานสัปดาห์ละ 60 ชั่วโมง... มันสร้างความแตกต่างมหาศาล หลังจากนั้น 8–9 ปี แทบไม่มีบั๊กเลย

  • ส่วนนี้ทำให้นึกถึง การตรวจสอบความครบถ้วน ในโค้ด TypeScript พยายามใช้อยู่เสมอ
    https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

    • satisfies never แบบใหม่กว่านั้นเหมาะกับงานนี้มาก และยังสะดวกแม้ในกรณีที่ชอบใช้เชน if else

    • คุณอาจชอบ ts-pattern ก็ได้

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

  • ถ้าดูเฉพาะกรณีที่ใส่ else ชัดเจนให้กับทุก if ที่ไม่ได้เล็กน้อยโดยสิ้นเชิง ก็สงสัยว่าโค้ดนี้จะเรียบง่ายขึ้นแค่ไหน หากผู้เขียน Kubernetes ออกแบบโดยยึด structural pattern matching แทนบล็อก if/else
    ภาษากระแสหลักหลายภาษาที่รองรับ structural pattern matching มีเครื่องมือที่ตรวจสอบได้ตอนคอมไพล์ว่า matching ครบถ้วนหรือไม่ และเพียงเท่านั้นก็อาจเป็นวิธีที่เป็นธรรมชาติในภาษา พร้อมเพิ่มความหนาแน่นของข้อมูลในโค้ดได้

  • การอภิปรายเมื่อปี 2018: https://news.ycombinator.com/item?id=18772873

  • แค่กวาดตาดูโค้ดคร่าว ๆ แต่พูดตรง ๆ มันไม่ได้ดูแย่ขนาดนั้น มีบางส่วนที่ผมคงทำต่างออกไป แต่ผมเห็นโค้ดที่แย่กว่านี้มากมาเยอะแล้ว
    อย่างน้อยโค้ดนี้ก็ทำตามกฎหนึ่งอย่าง ทุกอย่างดูผ่านการคิดมาแล้ว และให้ความรู้สึกว่ามีระเบียบวิธีบางอย่างอยู่ในความวุ่นวายนี้ ผมเลือกโค้ดแบบนี้ได้ทุกเมื่อ มากกว่าจับฉ่ายแบบที่เห็นมาหลายครั้ง ซึ่งปะปนสไตล์กัน เขียนแบบขี้เกียจ และมีโครงสร้างไม่เป็นเหตุเป็นผล

  • สงสัยว่าทำไมถึงสร้างแนวปฏิบัติด้าน "ความปลอดภัย" ขึ้นมาใหม่ แต่กลับมองข้าม best practices ด้านวิศวกรรมซอฟต์แวร์ ที่มีเอกสารรองรับ
    โมดูลยาว 2,000 บรรทัด เมธอดยาว 200 บรรทัด และการซ้อน if 3–4 ชั้น มักถือว่าเป็นอันตราย คอมเมนต์ที่บอกแค่ว่าทำอะไร ไม่ใช่ทำไม ก็ไม่มีประโยชน์นัก และมีแนวโน้มคลาดเคลื่อนจากโค้ดจริง ยังเห็นการใช้ nil ที่ไม่จำเป็นด้วย แม้ยังไม่ลงลึกถึงปัญหาอย่าง coupling หรือหลัก single responsibility ก็เห็นประเด็นผิวเผินเหล่านี้แล้ว

    • ถ้าคิดว่าสิ่งเหล่านี้เป็นอันตราย แนะนำให้อ่าน "John Carmack on Inlined Code"
      http://number-none.com/blow/john_carmack_on_inlined_code.htm...

      "โค้ดควบคุมการบินของจรวด Armadillo มีแค่ไม่กี่พันบรรทัด ผมจึงจับฟังก์ชัน tic หลัก แล้วเริ่ม inline subroutine ทั้งหมด ผมพูดไม่ได้ว่าพบบั๊กที่ซ่อนอยู่ซึ่งอาจทำให้เกิดการตกจริง ๆ แต่พบตัวแปรบางตัวที่ถูกตั้งค่าหลายครั้ง และ control flow บางจุดที่ดูน่าสงสัยเล็กน้อย โค้ดสุดท้ายเล็กลงและสะอาดขึ้น"

      ถ้า Carmack เห็นคุณค่าในแนวทางนี้ ก็คงไม่ควรรีบปัดตก ความคิดเห็นต่อ ๆ มาก็น่าอ่านเช่นกัน

      "ในช่วงหลายปีหลังจากเขียนบทความนี้ ผมมอง pure functional programming ในแง่บวกขึ้นมาก แม้ใน C/C++ หากใช้อย่างสมเหตุสมผล... ถ้ามันเริ่มรับมือยาก ให้หาวิธีแยกบล็อกออกเป็น pure function"

    • บางครั้งก็เป็นกรณี "ไม่มีทางอื่น(TM)"
      การจำกัดจำนวนบรรทัดแบบตามอำเภอใจมักก่อให้เกิดการแตกเป็นชิ้นเล็กชิ้นน้อยโดยไม่จำเป็น พอรวม include, license, glue code และคอมเมนต์เข้าไปด้วย ก็กลายเป็น spaghetti ที่เข้าถึงยาก ลองพยายามรักษาเมธอดในโค้ดประสิทธิภาพสูงให้ไม่เกิน 200 บรรทัดดูสิ ประสิทธิภาพอาจร่วงลงเหมือนการบินของ Icarus ได้

เมื่ออ่านคอมเมนต์ในโค้ด จะเห็นว่าโค้ดนี้ถูกทำให้เรียบง่ายเป็นโมดูลเดียว และมีการใส่ know-how จำนวนมากเพื่อให้เข้าถึงได้ และที่สำคัญกว่านั้นคือดูแลต่อได้อย่างยั่งยืน สำหรับคนที่ไม่รู้ภาษา หรือไม่รู้ลอจิก คอมเมนต์ที่ช่วยให้เห็นภาพรวมว่าโค้ดทำอะไรนั้นมีประโยชน์มาก และอีก 6 เดือนให้หลัง แม้แต่โค้ดของตัวเองก็ยังดูแปลกหน้า ดังนั้นมันจึงมีประโยชน์กับตัวเองด้วย

คอมเมนต์เป็นส่วนหนึ่งของโค้ดและ codebase ถ้าแก้โค้ดรอบ ๆ แล้วไม่อัปเดตคอมเมนต์ไปด้วย ก็เท่ากับใส่ bug ด้านเอกสารลงไปในโค้ด การที่ compiler ไม่ประมวลผลมัน ไม่ได้แปลว่ามันไม่ใช่ส่วนเชิงฟังก์ชัน โดยเนื้อแท้แล้ว คอมเมนต์คือความรู้และเป็นบันทึกการวิจัยที่ฝังอยู่ในโค้ด และในการบำรุงรักษาโค้ดที่เขียนไว้ มันอาจมีค่ามากกว่าโค้ดที่ถูก execute เสียอีก

best practice ไม่ใช่กฎหมายหรือกฎตายตัว แต่เป็นแนวทาง ควรนำมาใช้เมื่อเหมาะกับ codebase ไม่ใช่ทำตามแบบไม่คิดจนสร้าง codebase ที่มีปัญหา บางครั้งก็ต้องยืดหยุ่นกฎและกำหนดขึ้นเอง และถ้าคุณรู้ว่ากำลังทำอะไรอยู่ นั่นก็เป็นสิ่งที่ยอมรับได้โดยสิ้นเชิง

  • เคยเขียนด้วยวิธีที่ “ปลอดภัย” แบบนี้มาค่อนข้างนาน แต่กลับสร้าง bug มากกว่าการจัดการข้อผิดพลาดแบบ railway-oriented ผ่าน early return มาก และยังใช้เวลาแก้นานกว่ามากด้วย
    ถ้าใส่ else อย่างชัดเจนให้ทุกบล็อก if ความซับซ้อนจากการต้องจำบริบทปัจจุบันจะพุ่งสูงมาก ผมคิดว่าควรปรับกฎนี้เป็น “ทุกบล็อกเงื่อนไข if ต้อง early return หรือมีบล็อก else ที่สอดคล้องกัน” จะสมเหตุสมผลกว่า แพตเทิร์น if (cond) { การจัดการพิเศษ } ทำให้เสี่ยงกว่าและให้เหตุผลตามได้ยากกว่า early return อย่างชัดเจน

  • ไม่มีสิ่งที่เรียกว่า ชุด best practice ที่เป็นมาตรฐานเพียงหนึ่งเดียว
    ความยาวของฟังก์ชันหรือจำนวนบรรทัดโค้ดในไฟล์เองไม่ได้เป็นโทษหรือเป็นประโยชน์โดยเนื้อแท้ แต่ละภาษามีมุมมองของตัวเองว่าควรจัดโครงสร้างโค้ดอย่างไร แต่ไม่มีสิ่งใดอ้างได้ว่าเป็น “แนวปฏิบัติที่ดีที่สุด” Go ไม่ใช่ภาษาที่นิยมการแบ่งโค้ดออกเป็นไฟล์เล็ก ๆ จำนวนมาก

  • เมธอดยาว 200 บรรทัดไม่ได้ผิดโดยเนื้อแท้ หากโค้ดภายในเป็นเส้นตรงและรักษาระดับ abstraction เดียวกันไว้ได้ ก็อาจเป็นตัวเลือกที่ดีที่สุด
    ทางเลือกอย่างการสร้างเมธอด 5 บรรทัดจำนวน 40 เมธอดอาจแย่กว่า เพราะถ้าจะเข้าใจภาพรวมต้องกระโดดไปมาหลายที่ และอาจทำลำดับการเรียกผิดได้ จำนวน permutation ที่เป็นไปได้มีถึง 40! แบบ

  • โค้ดแบบนี้ดูเหมือนเป็นตัวเลือกที่เหมาะมากสำหรับย้ายไปเป็น ระบบเชิงประกาศ อิงกฎ และขับเคลื่อนด้วยตาราง
    วิธีนั้นเข้าใจและตรวจสอบได้ง่ายกว่าโค้ด imperative แบบเฉพาะกิจที่เต็มไปด้วย clause if โค้ดสกปรกประเภทนี้มักเป็นสัญญาณว่ามี abstraction ที่ขาดหายไป

    • แนวคิดของ Go โดยพื้นฐานแล้วใกล้เคียงกับการเขียนทุกอย่างลงไปแบบตรงไปตรงมา ราวกับย้ายโค้ดที่เดิมจะเขียนด้วย C มา และไม่พยายาม abstraction อะไรบางอย่าง