Não simplifique este código

• O pv_controller.go do Kubernetes é o controlador que sincroniza o binding de PV/PVC e, já no topo do arquivo, deixa explícito: “não simplifique e mantenha o estilo space shuttle”
• Esse estilo mantém um else correspondente para todo if e deixa comentários até em condições que parecem óbvias, para expor no código os ramos revisados e a intenção
• O centro do design são os ponteiros bidirecionais conectados por pvc.Spec.VolumeName e pv.Spec.ClaimRef, permitindo lidar com recuperação de concorrência, deleção, edições do usuário e bindings simultâneos em um ambiente sem transações
• O controlador combina observação de mudanças em PV/PVC, cache interno, fila com um único worker, registro de eventos, provisionamento dinâmico e interface de migração CSI para gerenciar as transições de estado do binding
• Os ramos verbosos e os comentários existem para preservar o conhecimento de negócio do comportamento e o contexto de recuperação de falhas, então mudanças futuras também devem seguir esse estilo

---

Papel e princípios de escrita de pv_controller.go

• pv_controller.go é o arquivo de implementação do PersistentVolumeController no pacote persistentvolume do Kubernetes
• Esse controlador mantém alinhado o estado de PersistentVolumeClaim e PersistentVolume
  • Controlador de cache que observa mudanças em PersistentVolume
  • Controlador de cache que observa mudanças em PersistentVolumeClaim
  • Sincronização do estado de PV/PVC com base nos eventos de mudança dos dois objetos
• O comentário no topo do arquivo alerta repetidamente para não simplificar esse código
  • O nome do estilo é space shuttle style
  • A abordagem é ter um else correspondente para toda instrução if
  • O objetivo é deixar todos os ramos explícitos, exceto checagens de erro simples
  • Mesmo comportamentos que parecem óbvios são comentados para que quem faz manutenção consiga acompanhar a complexidade do binding

Por que manter o space shuttle style

• Esse controlador é o resultado da unificação de trabalhos que originalmente estavam divididos em três controladores
• Ao simplificar o subsistema de PV, surgiu a necessidade de tratar todas as condições de forma explícita no código
• Como resultado, o código pode parecer verboso e cheio de comentários e ramificações
• Essa verbosidade é um mecanismo para deixar no código o conhecimento de negócio e o contexto do comportamento de binding
• Ao alterar esse arquivo, é preciso preservar o space shuttle style e, quando necessário, adicionar novos ramos e comentários da mesma forma

Design central: ponteiros bidirecionais entre PV e PVC

• No centro do design há ponteiros bidirecionais entre PV e PVC
  • Ponteiro do lado do PVC: pvc.Spec.VolumeName
  • Ponteiro do lado do PV: pv.Spec.ClaimRef
• Essa bidirecionalidade é difícil de tratar em um sistema sem transações, mas é necessária para garantir funcionamento correto mesmo em situações de falha
• Se uma instância HA rogue do controlador criar uma condição de corrida, podem surgir múltiplos bindings indistinguíveis, com possibilidade de perda de dados
• O controlador foi projetado principalmente para operar em modo de alta disponibilidade active-passive
  • As transições de objeto também foram projetadas para funcionar em HA active-active
  • Mas, se dois controladores ativos entrarem em conflito com frequência, o desempenho pode cair

Forma de binding e condições de recuperação

• O controlador suporta objetos pre-bound bidirecionais
  • Um PVC que deseja um PV específico
  • Um PV reservado para um PVC específico
• O binding ocorre em duas etapas
  • Primeiro, altera PV.Spec.ClaimRef
  • Depois, altera PVC.Spec.VolumeName
• Em qualquer ponto desse processo, o PV ou o PVC podem ser modificados ou apagados pelo usuário ou por outro controlador
• Também é possível que dois ou mais controladores tentem fazer binding simultâneo entre volumes e claims diferentes
• O controlador precisa ser capaz de recuperar desses cenários de conflito

Principais componentes da struct do controlador

• PersistentVolumeController possui listers, funções de sync de informer, cliente Kubernetes, registrador de eventos, gerenciador de plugins de volume e outros componentes necessários para sincronizar PV/PVC
• As últimas versões conhecidas de PV/PVC ficam armazenadas em cache interno
  • volumes persistentVolumeOrderedIndex
  • claims cache.Store
• Esse cache reflete tanto a versão mais recente salva no servidor de API quanto a versão recebida por eventos do etcd
• Um binding pode gerar cerca de quatro eventos
  • Atualização de volume.Spec
  • Atualização de volume.Status
  • Atualização de claim.Spec
  • Atualização de claim.Status
• Sem o cache interno, o informer pode manter um estado antigo e tentar corrigir novamente um binding que já foi concluído
• Se isso tentar regravar no servidor de API, pode ocorrer conflito de versão com o objeto já persistido

Fila de trabalho e restrições de concorrência

• O controlador tem workqueues separadas para processar claims e volumes
  • claimQueue
  • volumeQueue
• Cada fila deve ter exatamente uma worker thread
• Em especial, syncClaim() não é reentrante
• Se duas execuções de syncClaim() rodarem ao mesmo tempo, podem surgir os seguintes problemas
  • Fazer binding de dois claims diferentes ao mesmo volume
  • Fazer binding de um único claim a dois volumes
• O controlador consegue se recuperar dessas situações com erros de versão do servidor de API e verificações próprias, mas um modelo com múltiplos workers pode reduzir a velocidade geral

syncClaim: ponto de entrada da sincronização de PVC

• syncClaim é o método principal chamado quando um claim é criado, atualizado ou sincronizado periodicamente
• Esse método não diferencia o tipo de evento
• Primeiro, define a migration annotation correta no PVC e, se necessário, atualiza o servidor de API
• Em seguida, faz o desvio com base na presença da annotation AnnBindCompleted
  • Se a annotation não existir, chama syncUnboundClaim
  • Se a annotation existir, chama syncBoundClaim
• O processamento real é dividido entre métodos para claims unbound e bound por motivos de legibilidade

checkVolumeSatisfyClaim: verificação dos requisitos do PV

• checkVolumeSatisfyClaim verifica se o PV solicitado atende aos requisitos do PVC
• As condições verificadas são listadas explicitamente no código
  • Erro se o PV tiver DeletionTimestamp
  • Erro se a capacidade do PV for menor que a capacidade solicitada pelo PVC
  • Erro se storageClassName for diferente
  • Se o feature gate VolumeAttributesClass estiver ativado, verifica se VolumeAttributesClassName corresponde
  • Se o feature gate estiver desativado, mas houver VolumeAttributesClassName no claim ou no volume, gera erro
  • Erro se volumeMode não for compatível
  • Erro se o modo de acesso não for compatível
• Se todas as condições forem satisfeitas, retorna nil

Tratamento de eventos para PVC com delayed binding

• emitEventForUnboundDelayBindingClaim gera um evento informativo para claim não bound em modo de delayed binding
• O reason padrão é WaitForFirstConsumer
• A mensagem padrão diz que o binding ficará aguardando até que o primeiro consumer seja criado
• Se houver um Pod ainda não agendado que referencie esse PVC, o reason muda para WaitForPodScheduled
  • Se houver vários Pods, a mensagem inclui o nome de todos eles
  • No volume scheduling, só um Pod é considerado, mas como não se sabe qual será usado, todos os Pods são incluídos

syncUnboundClaim: tratamento de PVC ainda não bound

• Se claim.Spec.VolumeName estiver vazio, o usuário não pediu um PV específico
• Nesse caso, o controlador verifica o modo de delayed binding do claim e busca o PV mais adequado com findBestMatchForClaim
• Se não houver PV adequado, o processamento segue esta ordem
  • Se for possível atribuir uma StorageClass padrão, atualiza o PVC e encerra a sincronização
  • Se for delayed binding e ainda não estiver em estado de provisioning, gera um evento de espera
  • Se o claim tiver StorageClass, tenta provisionamento dinâmico com provisionClaim
  • Caso contrário, registra um evento FailedBinding dizendo que não há PV disponível nem StorageClass
• Se houver um PV adequado, chama bind para fazer o binding entre PV e PVC
  • Em caso de sucesso, registra métricas da operação de provision + binding e limpa o cache de timestamp
  • Se ocorrer erro ao salvar, um syncClaim posterior concluirá o binding

Tratamento de PVC que pede um PV específico

• Se claim.Spec.VolumeName não estiver vazio, o usuário pediu um PV específico
• Se o PV solicitado não estiver no cache, o status do PVC é atualizado para Pending e será tentado novamente depois
• Se o PV solicitado existir e volume.Spec.ClaimRef estiver vazio, o PV ainda não foi claimado
  • Verifica os requisitos com checkVolumeSatisfyClaim
  • Se os requisitos não forem atendidos, registra evento VolumeMismatch e mantém o PVC em Pending
  • Se os requisitos forem atendidos, chama bind
• Se o PV solicitado já estiver claimado por esse mesmo PVC, chama bind para concluir o binding
• Se o PV solicitado estiver ligado a outro claim, o tratamento é o seguinte
  • Se o claim não tiver annotation de binding feita pelo controlador, registra evento FailedBinding e o mantém em Pending
  • Se parecer ter sido bound pelo controlador, mas estiver ligado a outro claim, retorna erro em um estado “should never happen”

syncBoundClaim: tratamento de PVC já bound

• syncBoundClaim trata PVCs com a annotation AnnBindCompleted
• Se o claim já estiver bound, mas claim.Spec.VolumeName estiver vazio, muda o estado do claim para ClaimLost
  • A mensagem do evento diz que o claim bound perdeu a referência ao PV e os dados do volume foram perdidos
• Se o PV apontado pelo claim não existir, também muda para ClaimLost
  • A mensagem do evento diz que o claim bound perdeu o PersistentVolume e que os dados foram perdidos
• Se o PV existir, mas volume.Spec.ClaimRef estiver vazio, considera que o volume voltou ao estado unbound e chama bind novamente
• Se ClaimRef.UID do PV for igual ao UID do claim, considera binding normal e chama bind
  • Na maioria dos casos, essa chamada não faz nada
• Se o PV apontar para outro claimant, define a fase do claim como Lost, um estado terminal

syncVolume: ponto de entrada da sincronização de PV

• syncVolume é o método principal chamado quando um volume é criado, atualizado ou sincronizado periodicamente
• O tipo de evento não é diferenciado
• Primeiro, define no PV a migration annotation e o finalizer corretos e, se necessário, atualiza o servidor de API
• Se volume.Spec.ClaimRef estiver vazio, o volume é considerado não utilizado e sua fase vira Available
• Se ClaimRef existir, mas o UID estiver vazio, o PV é tratado como reservado para um PVC específico e sua fase vira Available
  • Nessa situação, o PVC ainda não está bound a esse PV, e o sync do PVC fará o processamento

Tratamento de PV quando o claim não é encontrado

• Se o PV estiver bound a um claim, o controlador procura o PVC pelo namespace/name em ClaimRef
• Quando o PVC não é encontrado no cache, ele faz verificações adicionais em certas condições
  • Verifica novamente no cache do informer
  • Verifica novamente no servidor de API
• Em PVs criados por provisioner externo ou binder externo, sob carga elevada, o PVC ainda pode não ter sido sincronizado para o cache local
• Para evitar fazer reclaim incorreto do PVC, é feito um duplo check
• Se for concluído que o claim não existe, a fase do volume muda para Released e reclaimVolume é executado
  • Se a fase anterior era Failed, ela não é sobrescrita
  • Se a política de reclaim for Retain, é registrado um log dizendo que o PV referencia um claim inexistente

Quando a conexão entre PV e PVC está desalinhada

• Se o claim existir, mas claim.Spec.VolumeName estiver vazio, o PVC ainda não tem o nome do PV
• Se volumeMode não bater, registra eventos VolumeMismatch tanto no PV quanto no PVC e ignora syncClaim
• Se não houver mismatch, adiciona o claim à claimQueue para que syncClaim seja chamado em breve
  • Essa abordagem acelera o binding de volumes provisionados
• Se claim.Spec.VolumeName for igual ao nome do volume atual, considera binding normal e atualiza a fase do volume para Bound
• Se o claim estiver bound a outro volume, o tratamento depende do caso
  • Se o volume tiver sido provisionado dinamicamente e a política de reclaim for Delete, marca como Released e executa reclaimVolume
  • Se for um volume bound pelo controlador, limpa com unbindVolume
  • Se o ponteiro tiver sido criado pelo usuário, mantém o ponteiro, mas chama unbindVolume para atualizar a fase e limpar ClaimRef.UID

Atualização de status e emissão de eventos

• updateClaimStatus salva o status do PVC no servidor de API
  • Mudança de fase
  • Quando não há volume, inicializa AccessModes, Capacity e CurrentVolumeAttributesClassName
  • Quando há volume, atualiza modo de acesso, capacidade e nome atual da classe de atributos do volume
• Há uma condição para atualizar a capacidade apenas no momento em que o claim vira Bound
  • A diferença entre o tamanho de filesystem do PVC e o tamanho do block device do PV pode ser intencional, então a capacidade de um claim já bound não deve ser sobrescrita
• Se o feature gate VolumeAttributesClass estiver ativado, CurrentVolumeAttributesClassName é definido na transição de pending para bound
  • Depois disso, um resizer ou override administrativo deve lidar com o valor; se o controlador continuar definindo esse campo, pode haver race condition
• updateClaimStatusWithEvent e updateVolumePhaseWithEvent só emitem eventos quando o status/fase realmente muda

Atribuição da StorageClass padrão

• assignDefaultStorageClass procura e atribui a StorageClass padrão quando o claim não tem storage class
• Claims que já têm storage class são ignorados
• Se não houver classe padrão, nada é atualizado e a função retorna false
• Se houver classe padrão, define o nome da classe em claim.Spec.StorageClassName e atualiza o servidor de API

Escopo do arquivo e limites explícitos

• Segundo os metadados do arquivo exibidos no GitHub, pv_controller.go tem 2038 linhas, 1864 LOC e 91 KB
• O corpo fornecido inclui apenas da parte inicial do arquivo até o começo da função bindVolumeToClaim; o restante continua por um link para a visualização raw
• Portanto, este resumo se limita à estrutura do controlador, aos comentários de design, aos principais ramos de sincronização e à lógica de atualização de estado visíveis no trecho de código fornecido

[GN⁺]

Opiniões no Hacker News

• Não sei se é estranho que o código neste arquivo realmente pareça código Go comum. Por ser Go, é verboso e, por não depender de abstrações profundas, parece mais longo, mas o código em si me parece típico.
  Abstração é uma faca de dois gumes, então esse jeito também é válido; se não houvesse o prefácio, acho que eu nem teria pensado duas vezes sobre o estilo de escrita. Talvez a diferença venha de eu ter mais experiência com software corporativo do que com software de sistemas. Para alguém que contribui constantemente com Kubernetes, esses comentários podem parecer desnecessários, mas, se fosse código que um leitor de um ambiente corporativo fosse ler em um futuro distante sem contexto, nessa complexidade eu provavelmente teria colocado ainda mais comentários

  • Antigamente esse tipo de código parecia normal, mas, nos últimos 10 anos mais ou menos, parece que muita gente passou a valorizar concisão acima de explicitude.
    Especialmente em código importante como esse, prefiro muito mais explicitude. Várias vezes na minha carreira, por causa de código que combinava várias condições e omitia comentários explicando o contexto de negócio e o significado, não dava para saber se o comportamento atual era intencional ou acidental. Esse estilo tende a virar código que impede mudanças, não código resistente a mudanças, e no mínimo torna difícil para alguém que não seja o autor alterá-lo. Criar uma cerca de Chesterton desnecessária vai contra a manutenibilidade

  • Esse comentário provavelmente foi adicionado depois de uma tentativa fracassada de simplificar o código, como um aviso para futuros mantenedores pensarem de novo antes de tentar a mesma coisa.
    O commit que adicionou o aviso foi "Add note about space-shuttle code style"[1], e o commit imediatamente anterior foi "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...

  • Eu estava pensando de forma parecida, mas mudei de ideia quando vi os ifs muito aninhados. Ali eu certamente teria criado ramificações com retorno antecipado.
    Dá a sensação de que terminaram só a primeira etapa de "faça funcionar, faça ficar rápido, faça ficar bonito" e não fizeram a parte de "deixar bonito". Já escrevi código feio e cheio de comentários assim ao destrinchar interações complicadas de estado, mas normalmente arrumo um pouco antes da revisão. Talvez fosse melhor simplesmente colocar um grande banner no topo do arquivo dizendo "não tente simplificar este código". Ainda assim, com certeza não é terrível

  • Pode até ser estranho, mas você não está sozinho. Para mim, esse código parece totalmente normal. Já escrevi código e comentários desse tipo em componentes que eu considerava importantes para a confiabilidade do sistema.
    Nunca concordei com a moda de "código sem comentários", e, quando voltei meses ou anos depois, os comentários que eu mesmo tinha escrito foram valiosos para o meu eu do futuro com muita frequência. É difícil imaginar reconstruir a lógica embutida em um componente com esse grau de complexidade sem comentários sólidos

  • Em especial, a afirmação de que todo if tem um comentário de else correspondente não parece ser sempre verdadeira. Muitos dos ifs sem correspondente são verificações simples como if (err != nil) { ou outros casos de retorno antecipado, mas, mesmo excluindo esses, parece haver ifs sem correspondente.
    Dito isso, pela minha experiência com software corporativo, comentários adicionais também não eram necessariamente tão numerosos. A base de código tinha comentários // end if como uma praga, mas comentários explicativos de verdade eram raros

• Texto sobre a qualidade do software do Space Shuttle: https://archive.is/HX7n4
  Em resumo, o que torna esse software incrível não é o quanto ele faz, mas o quão bem ele funciona. Dizem que ele nunca trava, não precisa ser reiniciado, não tem bugs e é o mais próximo da perfeição que humanos já alcançaram. As últimas três versões tinham 420 mil linhas cada, com apenas um erro em cada uma, e houve 17 erros no total nas últimas 11 versões. Dizem que um programa comercial de complexidade semelhante teria algo como 5.000 erros

  • Fico curioso sobre o que exatamente significa dizer que "as últimas três versões tinham 420 mil linhas cada e um erro em cada uma". Se cada uma das três versões tinha exatamente um bug, isso não é uma forma estranha de dizer que as duas correções anteriores não funcionaram ou introduziram um novo bug?
  • Seria interessante comparar em que o método da NASA difere do método da SpaceX. Como a SpaceX também realizou missões tripuladas, os requisitos parecem bastante semelhantes
  • 5000 / 17 ≈ 295. Seria justo presumir que um programa comercial de complexidade semelhante recebeu 295 vezes menos pessoa-hora?
  • O problema da metodologia de desenvolvimento do Space Shuttle é que ela é absurdamente cara e lenta, mas ainda assim não é 100% livre de bugs.
    Por ser cara e lenta demais, provar a correção do software com assistentes de prova modernos provavelmente seria muito mais barato, rápido e, na prática, mais seguro. Projetos como seL4 e CompCert mostram como fazer isso
  • É um dos meus textos favoritos. É incrível que um texto da internet de 1996 ainda esteja acessível

• Entendo a intenção de // KEEP THE SPACE SHUTTLE FLYING., mas é meio engraçado fazer referência, em um comentário, a um sistema que não está mais em operação por causa de um histórico de segurança ruim.
  Será que daqui a uns 10 anos as pessoas ainda vão se lembrar bem do Space Shuttle?

  • Os problemas de segurança do Space Shuttle foram, em grande parte, problemas de hardware, não de software.
    No "Appendix F - Personal Observations on Reliability of Shuttle" [0], apêndice de Richard Feynman no relatório do acidente do Challenger de 1986, ele diz o seguinte:

    Em resumo, o sistema e a atitude de verificação do software de computador são da mais alta qualidade. Não se observa o processo gradual de autoengano por meio do rebaixamento de critérios que é característico dos sistemas de segurança do Solid Rocket Booster ou do Space Shuttle Main Engine.

    Ele destacou especificamente a qualidade do software aviônico como exemplo de que até grandes projetos governamentais complexos como o Shuttle podem ser bem engenheirados, e que eles não estão, por si só, destinados a baixa qualidade e risco

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

• Levou pessoas e equipamentos ao espaço e os trouxe de volta para casa em bem mais de 100 missões bem-sucedidas. Eu ainda vejo isso de forma positiva, e é bem provável que continue vendo. Em termos de progresso humano e efeito líquido, foi um sucesso.

• O que encerrou o Shuttle não foi um histórico de segurança ruim, mas custo e a expectativa de uma piora futura na segurança.
  Embora os dois acidentes do Shuttle tenham matado mais astronautas do que outros desastres da NASA, considerando a dificuldade do que de fato aconteceu, o histórico de segurança foi realmente impressionante. O código parece muito bom.

• A situação do Space Shuttle é mais complexa do que simplesmente dizer que sua segurança era ruim. Em termos de missões, seu histórico é melhor que o de outros veículos lançadores. O Shuttle teve 2 missões fatais em 135; a Soyuz da era soviética teve 2 em 66; e o SpaceShipTwo tem um histórico assustadoramente ruim, com 1 missão fatal em apenas 12 voos.
  Porém, o Space Shuttle tinha uma capacidade de tripulação muito maior do que a necessária para a maioria das missões. Ao contrário da Apollo ou da Soyuz, com 3 pessoas, ele podia levar até 8; e, considerando que a maioria das missões soviéticas/Roscosmos, ESA e CNSA eram missões autônomas totalmente não tripuladas, não havia tripulação alguma a ser colocada em risco. Talvez essa analogia se encaixe melhor no Kubernetes: um sistema altamente engenheirado, poderoso e multiuso, mas que exige muita atenção e provavelmente é usado um pouco mais do que o necessário.

• Pelo critério mais comum, por passageiro-milha, o Space Shuttle está entre os veículos mais seguros já construídos e voados.
  Falando francamente como alguém cuja infância foi exatamente nos anos 1980: não sei como seria possível não lembrá-lo com carinho. Será que você é jovem demais, vê esse programa e todas as suas missões e conquistas apenas em retrospecto, e tem uma perspectiva contaminada pelo clima atual de uma era centrada em empreiteiras espaciais privadas?

• A história de Richard Hipp sobre adequar o código do SQLite a padrões de aviação também é bastante interessante: https://corecursive.com/066-sqlite-with-richard-hipp/#testin...

  DO-178B. É um padrão de qualidade para produtos aeronáuticos críticos para a segurança... Os testes precisam fazer com que cada operação de desvio no código binário resultante seja executada pelo menos uma vez e passe por cada caminho pelo menos uma vez... Levou um ano trabalhando 60 horas por semana... Fez uma diferença enorme. Nos 8 ou 9 anos seguintes, praticamente não houve bugs.

• Esta parte me lembra a verificação de exaustividade em código TypeScript. Tento usá-la sempre.
  https://www.typescriptlang.org/docs/handbook/2/narrowing.htm...

  • O satisfies never, mais recente, é muito bom para esse uso. Também é conveniente quando, por preferência, você usa cadeias de if else.

  • Talvez você goste de ts-pattern.

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

• Considerando apenas os casos em que se coloca um else explícito em cada if que não seja totalmente trivial, fico curioso sobre quanto esse código teria ficado mais simples se os autores do Kubernetes tivessem projetado tudo em torno de pattern matching estrutural, em vez de blocos if/else.
  Várias linguagens mainstream que oferecem pattern matching estrutural têm ferramentas para verificar em tempo de compilação se o matching é exaustivo, e só isso já poderia ser uma solução idiomática, aumentando a densidade de informação do código.

• Discussão de 2018: https://news.ycombinator.com/item?id=18772873

• Só dei uma olhada por alto no código, mas, sinceramente, ele não me parece tão ruim. Há coisas que eu teria feito de outro jeito, mas já vi muito código bem pior.
  Pelo menos este código segue uma regra, tudo parece ter sido escrito depois de pensar, e dá a impressão de que há um método nesse caos. Eu escolheria esse tipo de código a qualquer momento em vez da típica mistura confusa de estilos, programação preguiçosa e estrutura ilógica que já vi tantas vezes.

• Fico me perguntando por que, ao criar novas práticas de "segurança", ignoram boas práticas de engenharia de software documentadas.
  Módulos de 2.000 linhas, métodos de 200 linhas e 3 ou 4 níveis de aninhamento de if são considerados prejudiciais. Comentários que dizem apenas o que o código faz, e não por quê, também não são úteis e tendem a divergir do código real. Também se vê uso desnecessário de nil. Mesmo sem entrar em questões mais profundas, como acoplamento ou princípio da responsabilidade única, esses pontos aparecem na superfície.

  • Se você acha que essas coisas são prejudiciais, recomendo ler "John Carmack on Inlined Code".
    http://number-none.com/blow/john_carmack_on_inlined_code.htm...

    "O código de controle de voo dos foguetes da Armadillo tinha só alguns milhares de linhas, então peguei a função tic principal e comecei a inlinar todas as sub-rotinas. Não posso dizer que encontrei um bug oculto que pudesse causar uma queda real, mas encontrei algumas variáveis sendo definidas várias vezes e alguns fluxos de controle que pareciam um pouco suspeitos, e o código final ficou menor e mais limpo."

    Se Carmack encontrou valor nessa abordagem, acho que não devemos descartá-la apressadamente. Os comentários posteriores também valem a leitura.

    "Nos anos desde que escrevi isso, passei a ver com muito mais bons olhos, mesmo em C/C++, a programação funcional pura dentro de limites razoáveis... Quando ficar difícil de lidar, encontre uma forma de separar blocos em funções puras."

  • Às vezes é caso de "não há outro jeito(TM)".
    Limites arbitrários de número de linhas tendem a gerar fragmentação desnecessária. Some includes, licenças, código de cola e comentários, e vira um espaguete difícil de abordar. Tente manter métodos com 200 linhas em código de alto desempenho e a performance pode despencar como o voo de Ícaro.

Ao ler os comentários do código, dá para ver que uma enorme quantidade de know-how foi colocada ali para simplificar esse código em um único módulo e torná-lo acessível e, mais importante, sustentável. Para quem não conhece a linguagem ou a lógica, comentários que dão uma noção do que o código faz são muito úteis. Daqui a 6 meses, até o próprio código vai parecer estranho, então eles também são úteis para você mesmo.

Comentários fazem parte do código e da codebase. Se você altera o código ao redor e não atualiza os comentários junto, está inserindo um bug de documentação no código. O fato de o compilador não processá-los não significa que não sejam uma parte funcional. Em essência, comentários são conhecimento e notas de pesquisa embutidas no código, e podem ser mais valiosos na manutenção do código escrito do que o código que é executado.

Boas práticas são diretrizes, não leis nem regras rígidas. Devem ser aplicadas quando fizerem sentido para a codebase, não seguidas cegamente a ponto de criar uma codebase problemática. Às vezes é preciso flexibilizar regras e criar as suas próprias; se você sabe o que está fazendo, isso é totalmente aceitável.

• Escrevi por bastante tempo dessa forma "segura", mas acabei criando muito mais bugs do que com tratamento de erros em estilo ferroviário usando retorno antecipado, e também levei muito mais tempo para corrigi-los
  Colocar um else explícito em todo bloco if faz explodir a complexidade de ter que manter o contexto atual na cabeça. Acho razoável mudar essa regra para: "todo bloco condicional if retorna antecipadamente ou tem um bloco else correspondente". O padrão if (cond) { tratamento especial } certamente torna o código muito mais perigoso e difícil de raciocinar do que o retorno antecipado.

• Não existe um único conjunto canônico de boas práticas
  O tamanho de uma função ou o número de linhas de código em um arquivo não é, por si só, prejudicial nem benéfico. Cada linguagem tem sua própria visão sobre como organizar código, mas nada disso pode ser apresentado como "a melhor prática". Go não é uma linguagem que favorece dividir o código em muitos arquivos pequenos.

• Um método de 200 linhas não é inerentemente errado. Se o código interno for linear e mantiver o mesmo nível de abstração, pode ser a melhor escolha
  A alternativa de criar 40 métodos de 5 linhas pode ser pior. Para entender o todo, é preciso ficar pulando de um lado para o outro, e ainda dá para estragar a ordem das chamadas. Há 40! permutações possíveis.

• Esse tipo de código parece um candidato ideal para migrar para um sistema declarativo, baseado em regras e orientado por tabelas
  Essa abordagem é mais fácil de entender e verificar do que um monte de cláusulas if imperativas e improvisadas. Esse tipo de código bagunçado normalmente é sinal de que há uma abstração faltando.

  • A filosofia de Go é, basicamente, escrever tudo de forma parecida com uma transposição mais ou menos direta do código que você teria escrito em C, sem tentar abstrair as coisas.