fix bad bug whereby pending blocks would be removed before fully cert…

…ified. Yi Fix Test subjects in lifecycle tests to not generate genesis.
salesforce · May 25, 2024 · 6a2ef59 · 6a2ef59
1 parent ec56636
commit 6a2ef59
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 13 deletions.
diff --git a/choam/src/main/java/com/salesforce/apollo/choam/Producer.java b/choam/src/main/java/com/salesforce/apollo/choam/Producer.java
@@ -27,7 +27,10 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.util.*;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.ConcurrentSkipListMap;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -240,11 +243,12 @@ private void processAssemblies(List<UnitData> aggregate) {
     }
 
     private void processPendingValidations(HashedBlock block, PendingBlock p) {
-        var pending = pendingValidations.remove(block.hash);
+        var pending = pendingValidations.get(block.hash);
         if (pending != null) {
             pending.forEach(v -> validate(v, p, block.hash));
             if (p.witnesses.size() >= params().majority()) {
                 publish(p);
+                pendingValidations.remove(block.hash);
             }
         }
     }
@@ -254,13 +258,13 @@ private void processTransactions(boolean last, List<UnitData> aggregate) {
         final var txns = aggregate.stream().flatMap(e -> e.getTransactionsList().stream()).toList();
 
         if (txns.isEmpty()) {
-            if (preblocks % 5 == 0) {
-                pending.values()
-                       .stream()
-                       .filter(pb -> pb.published.get())
-                       .max(Comparator.comparing(pb -> pb.block.height()))
-                       .ifPresent(this::publish);
-            }
+            //            if (preblocks % 5 == 0) {
+            //                pending.values()
+            //                       .stream()
+            //                       .filter(pb -> pb.published.get())
+            //                       .max(Comparator.comparing(pb -> pb.block.height()))
+            //                       .ifPresent(pb -> publish(pb, true));
+            //            }
             return;
         }
         log.trace("transactions: {} combined hash: {} height: {} on: {}", txns.size(),
@@ -306,10 +310,20 @@ private void produceAssemble(ViewAssembly.Vue v) {
     }
 
     private void publish(PendingBlock p) {
+        this.publish(p, false);
+    }
+
+    private void publish(PendingBlock p, boolean force) {
         assert p.witnesses.size() >= params().majority() : "Publishing non majority block";
-        log.debug("Published pending: {} hash: {} height: {} witnesses: {} on: {}", p.block.block.getBodyCase(),
-                  p.block.hash, p.block.height(), p.witnesses.values().size(), params().member().getId());
-        p.published.set(true);
+        var publish = p.published.compareAndSet(false, true);
+        if (!publish) {
+            log.warn("Already published: {} hash: {} height: {} witnesses: {} on: {}", p.block.block.getBodyCase(),
+                     p.block.hash, p.block.height(), p.witnesses.values().size(), params().member().getId());
+            return;
+        }
+        log.warn("Publishing {}pending: {} hash: {} height: {} witnesses: {} on: {}", force ? "(forced) " : " ",
+                 p.block.block.getBodyCase(), p.block.hash, p.block.height(), p.witnesses.values().size(),
+                 params().member().getId());
         final var cb = CertifiedBlock.newBuilder()
                                      .setBlock(p.block.block)
                                      .addAllCertifications(

diff --git a/sql-state/src/test/java/com/salesforce/apollo/state/AbstractLifecycleTest.java b/sql-state/src/test/java/com/salesforce/apollo/state/AbstractLifecycleTest.java
@@ -338,7 +338,9 @@ private CHOAM createChoam(Random entropy, Builder params, SigningMember m, boole
         SqlStateMachine up = new SqlStateMachine(m.getId(), url, new Properties(),
                                                  new File(checkpointDirBase, m.getId().toString()));
         updaters.put(m, up);
-
+        if (testSubject) {
+            params.setGenerateGenesis(false);
+        }
         params.getProducer().ethereal().setSigner(m);
         return new CHOAM(params.setSynchronizationCycles(testSubject ? 100 : 10)
                                .build(RuntimeParameters.newBuilder()