Skip to content

Commit

Permalink
reverted validatorIndexCache (#8354)
Browse files Browse the repository at this point in the history 
Reverted the changes to validatorIndexCache, we'll have to take a look again for full electra support.

Although the intent was to only index finalized validators, the result was to constantly be doing StateTransitions somehow to get indices, and the outcome was a serious slow down in the important state transitions that happen constantly, so as a starting point, this will put us back to releasable code that will be ok for Deneb.

Before reverting, it seemed to process 1 state transition every 20 seconds or so, and even just rebuilding a handful of states was taking 10 minutes or longer, which is not really going to work in production.

We'll also need to build out testing for this once we re-implement. Archive storage is already slow due to epoch processing, we can't afford to take this kind of increased performance hit.

I'm guessing the tracker may still be part of an eventual solution, haven't removed at this point but i've cleaned it out functionally... i probably need to clean that before any merge...

`BlockProcessorElectra` was changed to not use the cache to find validator index, as it's a simple operation, and it was causing problems.

Signed-off-by: Paul Harris <[email protected]>
  • Loading branch information
@rolfyone
rolfyone authored Jun 6, 2024
1 parent d76da3e commit f2054c3
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 77 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,4 @@ the [releases page](https://github.com/Consensys/teku/releases).
endpoints. In the cases where the block has been produced in the same beacon node, only equivocation validation will be done instead of the entire gossip validation.

### Bug Fixes
- Fixed performance degradation introduced in 24.4.0 regarding archive state retrieval time.
90 changes: 34 additions & 56 deletions ...a/tech/pegasys/teku/spec/datastructures/state/beaconstate/common/ValidatorIndexCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import com.google.common.annotations.VisibleForTesting;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import tech.pegasys.teku.bls.BLSPublicKey;
import tech.pegasys.teku.infrastructure.collections.cache.Cache;
import tech.pegasys.teku.infrastructure.collections.cache.LRUCache;
Expand All @@ -25,16 +27,15 @@
import tech.pegasys.teku.spec.datastructures.state.beaconstate.BeaconState;

public class ValidatorIndexCache {
private static final Logger LOG = LogManager.getLogger();
private final Cache<BLSPublicKey, Integer> validatorIndices;
private final AtomicInteger lastCachedIndex;

private static final int INDEX_NONE = -1;

private final AtomicInteger latestFinalizedIndex;
static final ValidatorIndexCache NO_OP_INSTANCE =
new ValidatorIndexCache(NoOpCache.getNoOpCache(), INDEX_NONE, INDEX_NONE);

private final Cache<BLSPublicKey, Integer> validatorIndices;
private final AtomicInteger latestFinalizedIndex;
private final AtomicInteger lastCachedIndex;

@VisibleForTesting
ValidatorIndexCache(
final Cache<BLSPublicKey, Integer> validatorIndices,
Expand All @@ -46,88 +47,65 @@ public class ValidatorIndexCache {
}

public ValidatorIndexCache() {
validatorIndices = LRUCache.create(Integer.MAX_VALUE - 1);
this.validatorIndices = LRUCache.create(Integer.MAX_VALUE - 1);
this.lastCachedIndex = new AtomicInteger(INDEX_NONE);
latestFinalizedIndex = new AtomicInteger(INDEX_NONE);
lastCachedIndex = new AtomicInteger(INDEX_NONE);
}

public Optional<Integer> getValidatorIndex(
final BeaconState state, final BLSPublicKey publicKey) {
final SszList<Validator> validators = state.getValidators();
final Optional<Integer> validatorIndex = validatorIndices.getCached(publicKey);
if (validatorIndex.isPresent()) {
return validatorIndex.filter(index -> index < validators.size());
return validatorIndex.filter(index -> index < state.getValidators().size());
}
// Using the same latestFinalizedIndex when scanning through
// the finalized and the non-finalized states ensures consistency
final int latestFinalizedIndexSnapshot = latestFinalizedIndex.get();
return findIndexFromFinalizedState(validators, publicKey, latestFinalizedIndexSnapshot)
.or(
() ->
findIndexFromNonFinalizedState(
validators, publicKey, latestFinalizedIndexSnapshot));

return findIndexFromState(state.getValidators(), publicKey);
}

private Optional<Integer> findIndexFromFinalizedState(
final SszList<Validator> validators,
final BLSPublicKey publicKey,
final int latestFinalizedIndex) {
for (int i = lastCachedIndex.get() + 1;
i <= Math.min(latestFinalizedIndex, validators.size() - 1);
i++) {
final BLSPublicKey pubKey = validators.get(i).getPublicKey();
// cache finalized mapping
private Optional<Integer> findIndexFromState(
final SszList<Validator> validatorList, final BLSPublicKey publicKey) {
for (int i = Math.max(lastCachedIndex.get() + 1, 0); i < validatorList.size(); i++) {
BLSPublicKey pubKey = validatorList.get(i).getPublicKey();
// from electra, only finalized validator indices are stable,
// so we look to store only finalized indices not already stored.
validatorIndices.invalidateWithNewValue(pubKey, i);
updateLastCachedIndex(i);
updateLastIndex(i);
if (pubKey.equals(publicKey)) {
return Optional.of(i);
}
}
return Optional.empty();
}

private void updateLastCachedIndex(final int updatedIndex) {
lastCachedIndex.updateAndGet(curr -> Math.max(curr, updatedIndex));
}

private Optional<Integer> findIndexFromNonFinalizedState(
final SszList<Validator> validators,
final BLSPublicKey publicKey,
final int latestFinalizedIndex) {
for (int i = latestFinalizedIndex + 1; i < validators.size(); i++) {
final BLSPublicKey pubKey = validators.get(i).getPublicKey();
if (pubKey.equals(publicKey)) {
return Optional.of(i);
}
}
return Optional.empty();
}

public void updateLatestFinalizedIndex(final BeaconState finalizedState) {
latestFinalizedIndex.updateAndGet(
curr -> Math.max(curr, finalizedState.getValidators().size() - 1));
private void updateLastIndex(final int i) {
lastCachedIndex.updateAndGet(curr -> Math.max(curr, i));
}

public void invalidateWithNewValue(final BLSPublicKey pubKey, final int updatedIndex) {
if (updatedIndex > latestFinalizedIndex.get()) {
// do not cache if index is not finalized
return;
if (latestFinalizedIndex.get() >= updatedIndex) {
validatorIndices.invalidateWithNewValue(pubKey, updatedIndex);
} else {
LOG.trace("Ignoring invalidateWithNewValue {}:{}", pubKey, updatedIndex);
}
validatorIndices.invalidateWithNewValue(pubKey, updatedIndex);
}

@VisibleForTesting
public int getLatestFinalizedIndex() {
return latestFinalizedIndex.get();
int getLastCachedIndex() {
return lastCachedIndex.get();
}

@VisibleForTesting
Cache<BLSPublicKey, Integer> getValidatorIndices() {
return validatorIndices;
}

@VisibleForTesting
int getLastCachedIndex() {
return lastCachedIndex.get();
public void updateLatestFinalizedIndex(final BeaconState finalizedState) {
LOG.trace("Update finalized index: {}", finalizedState.getSlot());
latestFinalizedIndex.updateAndGet(
curr -> Math.max(curr, finalizedState.getValidators().size() - 1));
}

public int getLatestFinalizedIndex() {
return latestFinalizedIndex.get();
}
}
12 changes: 11 additions & 1 deletion .../main/java/tech/pegasys/teku/spec/logic/versions/electra/block/BlockProcessorElectra.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,17 @@ protected void addValidatorToRegistry(
final Validator validator = getValidatorFromDeposit(pubkey, withdrawalCredentials);
LOG.debug("Adding new validator with index {} to state", state.getValidators().size());
state.getValidators().append(validator);
final int validatorIndex = validatorsUtil.getValidatorIndex(state, pubkey).orElseThrow();
int validatorIndex = -1;
for (int i = state.getValidators().size() - 1; i >= 0; i--) {
if (state.getValidators().get(i).getPublicKey().equals(pubkey)) {
validatorIndex = i;
break;
}
}
if (validatorIndex < 0) {
throw new IllegalStateException(
"Could not locate validator " + pubkey + " after adding to state.");
}
final MutableBeaconStateElectra stateElectra = MutableBeaconStateElectra.required(state);

stateElectra.getBalances().appendElement(UInt64.ZERO);
Expand Down
20 changes: 0 additions & 20 deletions ...ch/pegasys/teku/spec/datastructures/state/beaconstate/common/ValidatorIndexCacheTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand Down Expand Up @@ -98,25 +97,6 @@ public void shouldStartScanningFinalizedStateFromLastCachedIndex() {
assertThat(validatorIndexCache.getLastCachedIndex()).isEqualTo(latestFinalizedIndex);
}

@Test
public void shouldScanNonFinalizedStateButDoNotCacheIfIndexNotFoundInFinalizedState() {
final SszList<Validator> validators = state.getValidators();
final int latestFinalizedIndex = 31;
final ValidatorIndexCache validatorIndexCache =
new ValidatorIndexCache(cache, latestFinalizedIndex, latestFinalizedIndex);

when(cache.getCached(any())).thenReturn(Optional.empty());

final Optional<Integer> index =
validatorIndexCache.getValidatorIndex(
state, validators.get(NUMBER_OF_VALIDATORS - 1).getPublicKey());

verify(cache, never()).invalidateWithNewValue(any(), any());
assertThat(index).hasValue(NUMBER_OF_VALIDATORS - 1);

assertThat(validatorIndexCache.getLastCachedIndex()).isEqualTo(latestFinalizedIndex);
}

@Test
public void shouldReturnEmptyIfPubkeyNotFoundInState() {
final ValidatorIndexCache validatorIndexCache = new ValidatorIndexCache();
Expand Down

0 comments on commit f2054c3

Please sign in to comment.