From fcb7b9f356bf540875482ff55ade65fa20312929 Mon Sep 17 00:00:00 2001 From: Stefan Bratanov Date: Thu, 28 Nov 2024 10:38:52 +0000 Subject: [PATCH] Do tracking at beginning --- .../DefaultPerformanceTracker.java | 6 ++--- .../performance/NoOpPerformanceTracker.java | 4 ++-- .../performance/PerformanceTracker.java | 4 ++-- .../publisher/AbstractBlockPublisher.java | 12 ++++------ .../DefaultPerformanceTrackerTest.java | 23 ++++++++++++------- .../publisher/AbstractBlockPublisherTest.java | 3 ++- 6 files changed, 29 insertions(+), 23 deletions(-) diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java index eb9ea756cae..afce203d35f 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTracker.java @@ -420,11 +420,11 @@ public void saveProducedAttestation(final Attestation attestation) { } @Override - public void saveProducedBlock(final SignedBeaconBlock block) { - final UInt64 epoch = spec.computeEpochAtSlot(block.getSlot()); + public void saveProducedBlock(final SlotAndBlockRoot slotAndBlockRoot) { + final UInt64 epoch = spec.computeEpochAtSlot(slotAndBlockRoot.getSlot()); final Set blocksInEpoch = producedBlocksByEpoch.computeIfAbsent(epoch, __ -> concurrentSet()); - blocksInEpoch.add(block.getSlotAndBlockRoot()); + blocksInEpoch.add(slotAndBlockRoot); } @Override diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java index 69e28006dcc..988b07a8f62 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/NoOpPerformanceTracker.java @@ -15,7 +15,7 @@ import it.unimi.dsi.fastutil.ints.IntSet; import tech.pegasys.teku.infrastructure.unsigned.UInt64; -import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; +import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.versions.altair.SyncCommitteeMessage; @@ -28,7 +28,7 @@ public void start(final UInt64 nodeStartSlot) {} public void saveProducedAttestation(final Attestation attestation) {} @Override - public void saveProducedBlock(final SignedBeaconBlock block) {} + public void saveProducedBlock(final SlotAndBlockRoot slotAndBlockRoot) {} @Override public void reportBlockProductionAttempt(final UInt64 epoch) {} diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java index 955d3d45216..58aa7785781 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/performance/PerformanceTracker.java @@ -16,7 +16,7 @@ import it.unimi.dsi.fastutil.ints.IntSet; import tech.pegasys.teku.ethereum.events.SlotEventsChannel; import tech.pegasys.teku.infrastructure.unsigned.UInt64; -import tech.pegasys.teku.spec.datastructures.blocks.SignedBeaconBlock; +import tech.pegasys.teku.spec.datastructures.blocks.SlotAndBlockRoot; import tech.pegasys.teku.spec.datastructures.operations.Attestation; import tech.pegasys.teku.spec.datastructures.operations.versions.altair.SyncCommitteeMessage; @@ -26,7 +26,7 @@ public interface PerformanceTracker extends SlotEventsChannel { void saveProducedAttestation(Attestation attestation); - void saveProducedBlock(SignedBeaconBlock block); + void saveProducedBlock(SlotAndBlockRoot slotAndBlockRoot); void reportBlockProductionAttempt(UInt64 epoch); diff --git a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java index 2675fc3e3d9..d4df310a2d1 100644 --- a/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java +++ b/beacon/validator/src/main/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisher.java @@ -73,6 +73,10 @@ public SafeFuture sendSignedBlock( final SignedBlockContainer blockContainer, final BroadcastValidationLevel broadcastValidationLevel, final BlockPublishingPerformance blockPublishingPerformance) { + // always track the block as produced even in case of publishing failures (e.g. + // relay API timeouts during unblinding), because we later do comparison against the chain data + // anyway + performanceTracker.saveProducedBlock(blockContainer.getSignedBlock().getSlotAndBlockRoot()); return blockFactory .unblindSignedBlockIfBlinded(blockContainer.getSignedBlock(), blockPublishingPerformance) .thenCompose( @@ -84,13 +88,7 @@ public SafeFuture sendSignedBlock( Suppliers.memoize(() -> blockFactory.createBlobSidecars(blockContainer)), broadcastValidationLevel, blockPublishingPerformance)) - .thenCompose(result -> calculateResult(blockContainer, result, blockPublishingPerformance)) - .alwaysRun( - () -> - // always track the block as produced even in case of publishing failures (e.g. - // relay API timeouts during unblinding), because we later do comparison against the - // chain data anyway - performanceTracker.saveProducedBlock(blockContainer.getSignedBlock())); + .thenCompose(result -> calculateResult(blockContainer, result, blockPublishingPerformance)); } private SafeFuture diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java index 10bfb8bcc15..147f15568c8 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/performance/DefaultPerformanceTrackerTest.java @@ -90,8 +90,10 @@ void shouldDisplayPerfectBlockInclusion() { chainUpdater.updateBestBlock(chainUpdater.advanceChainUntil(10)); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(1))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(2))); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(1)); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(2)); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(1).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(2).getSlotAndBlockRoot()); performanceTracker.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE)); BlockPerformance expectedBlockPerformance = new BlockPerformance(UInt64.ZERO, 2, 2, 2); verify(log).performance(expectedBlockPerformance.toString()); @@ -103,7 +105,7 @@ void shouldDisplayBlockInclusionWhenProducedBlockIsChainHead() { final SignedBlockAndState bestBlock = chainUpdater.advanceChainUntil(2); chainUpdater.updateBestBlock(bestBlock); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(bestBlock.getSlot())); - performanceTracker.saveProducedBlock(bestBlock.getBlock()); + performanceTracker.saveProducedBlock(bestBlock.getBlock().getSlotAndBlockRoot()); performanceTracker.onSlot(lastSlot); BlockPerformance expectedBlockPerformance = new BlockPerformance(UInt64.ZERO, 1, 1, 1); verify(log).performance(expectedBlockPerformance.toString()); @@ -115,9 +117,12 @@ void shouldDisplayOneMissedBlock() { performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(1))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(2))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(3))); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(1)); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(2)); - performanceTracker.saveProducedBlock(dataStructureUtil.randomSignedBeaconBlock(3)); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(1).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(2).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + dataStructureUtil.randomSignedBeaconBlock(3).getSlotAndBlockRoot()); performanceTracker.onSlot(spec.computeStartSlotAtEpoch(UInt64.ONE)); BlockPerformance expectedBlockPerformance = new BlockPerformance(UInt64.ZERO, 3, 2, 3); verify(log).performance(expectedBlockPerformance.toString()); @@ -258,8 +263,10 @@ void shouldClearOldSentObjects() { chainUpdater.updateBestBlock(chainUpdater.advanceChainUntil(10)); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(1))); performanceTracker.reportBlockProductionAttempt(spec.computeEpochAtSlot(UInt64.valueOf(2))); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(1)); - performanceTracker.saveProducedBlock(chainUpdater.chainBuilder.getBlockAtSlot(2)); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(1).getSlotAndBlockRoot()); + performanceTracker.saveProducedBlock( + chainUpdater.chainBuilder.getBlockAtSlot(2).getSlotAndBlockRoot()); performanceTracker.saveProducedAttestation( spec.getGenesisSchemaDefinitions() .getAttestationSchema() diff --git a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java index 7d2f1abd7fb..986ddcaaa3e 100644 --- a/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java +++ b/beacon/validator/src/test/java/tech/pegasys/teku/validator/coordinator/publisher/AbstractBlockPublisherTest.java @@ -263,7 +263,8 @@ public void sendSignedBlock_shouldTrackBlockAsProducedEvenIfExceptionOccurs() { BroadcastValidationLevel.NOT_REQUIRED, BlockPublishingPerformance.NOOP); - verify(performanceTracker).saveProducedBlock(signedBlockContents.getSignedBlock()); + verify(performanceTracker) + .saveProducedBlock(signedBlockContents.getSignedBlock().getSlotAndBlockRoot()); } private SafeFuture prepareBlockImportResult(