Hacktoberfest 2026: los issues que los mantenedores marcaron para octubre, abiertos y aptos para principiantes. Explorar issues de Hacktoberfest

Follow-ups to #681: decision count for a stopped view, regression tests, and cleanups

Cerrado
  • ¿Algo no está bien?
#714 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
35/100
Tipo de issue
Error
Claridad
Bastante claro
Estado de actividad
Activo
Stack tecnológico
go
Área
distributed-systems

Línea de trabajo

Start by reading #681 and the controller and view changer code around Controller.decide, run, sync, and commitInFlightProposal; the issue does not name source or test file paths. Check completion_signal_test.go and trace how aborts interact with buffered decisions and in-flight Decide or Sync calls. Done means fixing the stopped-current-view decision count, adding tests that fail on the old behavior, and addressing the listed cleanups; the optional late-result recovery is a separate improvement.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

Written by Claude (Anthropic's AI assistant) on behalf of Hagar Meir.

These are the points left open in the review of #681 (avoid blocking on completion signals when the waiter has left). They should be done in a separate PR once #681 is merged.

1. Decision count when the deciding view is still current but stopped (possible small bug)

In Controller.decide, isRunningCurrentView(d.view) returns false in two different cases:

  • The view was replaced. Skipping all of the view bookkeeping is right.
  • The view is still current but already stopped, for example when the run loop handles abortViewChan before the decision buffered in decisionChan. The decision was delivered in this view, and the checkpoint moved to its sequence, but incrementCurrentDecisionsInView is skipped.

In the second case, if the node then syncs and finds nothing newer, it restarts the same view with a count that is one too low:

  • run calls changeView(c.getCurrentViewNumber(), vs.ProposalSeq, c.getCurrentDecisionsInView()), or
  • sync keeps newDecisionsInView = c.getCurrentDecisionsInView() when the latest decision is at the controller's sequence.

With leader rotation on, getLeaderID can then give this node a different leader from the other nodes, until the next view change resets the count.

This is not a regression: before #681, the send on deliverChan failed in the same case and the increment was skipped too.

Proposed fix: when d.view is the current view but stopped, still call incrementCurrentDecisionsInView, and skip only checkIfRotate/changeView and acquireLeaderToken, the steps that could race the pending view change. Add a test for it.

2. Regression tests that fail on the old code

The tests added in #681 (completion_signal_test.go) pass on the old code as well:

  • decide now only closes d.delivered, and close never blocks;
  • the in-flight tests call decideInFlight and syncInFlight directly, and their non-blocking sends never block either.

Tests that reproduce the original hangs would catch a regression:

  • controller: Decide returns through the view's abort channel while the decision is still buffered in decisionChan, and the run loop must keep processing later events;
  • view changer: commitInFlightProposal times out or is stopped, and the deferred inFlightView.Abort() must not hang on a late Decide or Sync from the in-flight view.
3. Inconsistent nil checks

decide checks d.delivered != nil, but Decide always sets it. Meanwhile a nil d.view panics in isRunningCurrentView. The two fields are always set together, so both checks can be removed.

4. Comments in decide
  • "the current view must not be rotated or lead because of it" is unclear. It could say that the leader token is not acquired.
  • It could also say why skipping acquireLeaderToken is fine for a replaced view: the new view's startView acquires the token when needed.
5. Optional: in-flight attempt result after a late decision

If the in-flight attempt times out or is stopped while the in-flight view is delivering its decision, the deferred Abort() waits for the delivery to finish, so the proposal was committed. commitInFlightProposal still returns false, and the node waits for the next view change. Since decideCh is buffered, a non-blocking receive on it after the abort could return true instead. This only affects how quickly the node recovers, not correctness.

Lenguaje dominante
Go
Estrellas
110
Forks
41
Merge medio
1 d 4 h
PR fusionados (30 d)
31

Preparar el entorno

Este proyecto no incluye contenedor de desarrollo, Dockerfile ni guía de contribución, así que la configuración corre por tu cuenta: empieza por su README y consulta nuestra guía para la primera contribución para los pasos generales.

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de hyperledger/SmartBFT

Todos los issues de hyperledger/SmartBFT

Issues similares

Más issues de Go

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.