Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Expose transaction count by type metrics for the layered txpool #6903

Merged
merged 2 commits into from
Apr 8, 2024
Merged

Expose transaction count by type metrics for the layered txpool #6903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fab7f92
Add tx count by type txpool metric
fab-10 Apr 8, 2024
6257d7f
Update CHANGELOG
fab-10 Apr 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
- Update Web3j dependencies [#6811](https://github.com/hyperledger/besu/pull/6811)
- Add `tx-pool-blob-price-bump` option to configure the price bump percentage required to replace blob transactions (by default 100%) [#6874](https://github.com/hyperledger/besu/pull/6874)
- Log detailed timing of block creation steps [#6880](https://github.com/hyperledger/besu/pull/6880)
- Expose transaction count by type metrics for the layered txpool [#6903](https://github.com/hyperledger/besu/pull/6903)

### Bug fixes
- Fix txpool dump/restore race condition [#6665](https://github.com/hyperledger/besu/pull/6665)
Expand Down
27 changes: 27 additions & 0 deletions .../src/main/java/org/hyperledger/besu/ethereum/eth/transactions/TransactionPoolMetrics.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
*/
package org.hyperledger.besu.ethereum.eth.transactions;

import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.ethereum.transaction.TransactionInvalidReason;
import org.hyperledger.besu.metrics.BesuMetricCategory;
import org.hyperledger.besu.metrics.ReplaceableDoubleSupplier;
Expand All @@ -27,6 +28,7 @@
import java.util.Map;
import java.util.function.DoubleSupplier;

import org.apache.commons.lang3.tuple.Pair;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -43,12 +45,15 @@ public class TransactionPoolMetrics {
private final LabelledMetric<Counter> rejectedCounter;
private final LabelledGauge spaceUsed;
private final LabelledGauge transactionCount;
private final LabelledGauge transactionCountByType;
private final LabelledGauge uniqueSenderCount;
private final LabelledMetric<Counter> expiredMessagesCounter;
private final Map<String, RunnableCounter> expiredMessagesRunnableCounters = new HashMap<>();
private final LabelledMetric<Counter> alreadySeenTransactionsCounter;
private final Map<String, ReplaceableDoubleSupplier> spaceUsedSuppliers = new HashMap<>();
private final Map<String, ReplaceableDoubleSupplier> transactionCountSuppliers = new HashMap<>();
private final Map<Pair<String, TransactionType>, ReplaceableDoubleSupplier>
transactionCountByTypeSuppliers = new HashMap<>();
private final Map<String, ReplaceableDoubleSupplier> uniqueSendersSuppliers = new HashMap<>();

public TransactionPoolMetrics(final MetricsSystem metricsSystem) {
Expand Down Expand Up @@ -97,6 +102,14 @@ public TransactionPoolMetrics(final MetricsSystem metricsSystem) {
"The number of transactions currently present in the layer",
"layer");

transactionCountByType =
metricsSystem.createLabelledGauge(
BesuMetricCategory.TRANSACTION_POOL,
"number_of_transactions_by_type",
"The number of transactions, of a specified type, currently present in the layer",
"layer",
"type");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to the type name, should we expose the EIP-2718 type number? We shouldn't use ordinal and should encode it in TransactionType but that has some weirdness for frontier types. May be best for a future PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I think it makes sense to explicitly add the EIP-2718 type number to the tx type, and expose it in the metric label.
About the use of type.ordinal(), it is only used internally to index the array of int for the counters, and not exposed outside.


uniqueSenderCount =
metricsSystem.createLabelledGauge(
BesuMetricCategory.TRANSACTION_POOL,
Expand Down Expand Up @@ -136,6 +149,20 @@ public void initSpaceUsed(final DoubleSupplier spaceUsedSupplier, final String l
});
}

public void initTransactionCountByType(
final DoubleSupplier spaceUsedSupplier, final String layer, final TransactionType type) {
transactionCountByTypeSuppliers.compute(
Pair.of(layer, type),
(unused, existingSupplier) -> {
if (existingSupplier == null) {
final var newSupplier = new ReplaceableDoubleSupplier(spaceUsedSupplier);
transactionCountByType.labels(newSupplier, layer, type.name());
return newSupplier;
}
return existingSupplier.replaceDoubleSupplier(spaceUsedSupplier);
});
}

public void initTransactionCount(
final DoubleSupplier transactionCountSupplier, final String layer) {
transactionCountSuppliers.compute(
Expand Down
24 changes: 17 additions & 7 deletions ...ava/org/hyperledger/besu/ethereum/eth/transactions/layered/AbstractTransactionsLayer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@

import org.hyperledger.besu.datatypes.Address;
import org.hyperledger.besu.datatypes.Hash;
import org.hyperledger.besu.datatypes.TransactionType;
import org.hyperledger.besu.ethereum.core.BlockHeader;
import org.hyperledger.besu.ethereum.core.Transaction;
import org.hyperledger.besu.ethereum.eth.transactions.BlobCache;
Expand All @@ -39,6 +40,7 @@
import org.hyperledger.besu.util.Subscribers;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -74,7 +76,7 @@ public abstract class AbstractTransactionsLayer implements TransactionsLayer {
private OptionalLong nextLayerOnAddedListenerId = OptionalLong.empty();
private OptionalLong nextLayerOnDroppedListenerId = OptionalLong.empty();
protected long spaceUsed = 0;

protected final int[] txCountByType = new int[TransactionType.values().length];
private final BlobCache blobCache;

protected AbstractTransactionsLayer(
Expand All @@ -91,6 +93,11 @@ protected AbstractTransactionsLayer(
metrics.initSpaceUsed(this::getLayerSpaceUsed, name());
metrics.initTransactionCount(pendingTransactions::size, name());
metrics.initUniqueSenderCount(txsBySender::size, name());
Arrays.stream(TransactionType.values())
.forEach(
type ->
metrics.initTransactionCountByType(
() -> txCountByType[type.ordinal()], name(), type));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using ordinal to track the external feels fragile, but if experimental tx types start showing up they may leave large gaps. However... it's what we express externally, so this looks like it will work. See my other comment on the metric.

this.blobCache = blobCache;
}

Expand All @@ -101,6 +108,7 @@ public void reset() {
pendingTransactions.clear();
txsBySender.clear();
spaceUsed = 0;
Arrays.fill(txCountByType, 0);
nextLayer.reset();
}

Expand Down Expand Up @@ -286,7 +294,7 @@ private void processAdded(final PendingTransaction addedTx) {
pendingTransactions.put(addedTx.getHash(), addedTx);
final var senderTxs = txsBySender.computeIfAbsent(addedTx.getSender(), s -> new TreeMap<>());
senderTxs.put(addedTx.getNonce(), addedTx);
increaseSpaceUsed(addedTx);
increaseCounters(addedTx);
metrics.incrementAdded(addedTx, name());
internalAdd(senderTxs, addedTx);
}
Expand Down Expand Up @@ -332,7 +340,7 @@ private void evict(final long spaceToFree, final int txsToEvict) {

protected void replaced(final PendingTransaction replacedTx) {
pendingTransactions.remove(replacedTx.getHash());
decreaseSpaceUsed(replacedTx);
decreaseCounters(replacedTx);
metrics.incrementRemoved(replacedTx, REPLACED.label(), name());
internalReplaced(replacedTx);
notifyTransactionDropped(replacedTx);
Expand Down Expand Up @@ -368,7 +376,7 @@ protected PendingTransaction processRemove(
final PendingTransaction removedTx = pendingTransactions.remove(transaction.getHash());

if (removedTx != null) {
decreaseSpaceUsed(removedTx);
decreaseCounters(removedTx);
metrics.incrementRemoved(removedTx, removalReason.label(), name());
internalRemove(senderTxs, removedTx, removalReason);
}
Expand All @@ -381,7 +389,7 @@ protected PendingTransaction processEvict(
final RemovalReason reason) {
final PendingTransaction removedTx = pendingTransactions.remove(evictedTx.getHash());
if (removedTx != null) {
decreaseSpaceUsed(evictedTx);
decreaseCounters(evictedTx);
metrics.incrementRemoved(evictedTx, reason.label(), name());
internalEvict(senderTxs, removedTx);
}
Expand Down Expand Up @@ -467,12 +475,14 @@ protected abstract void internalRemove(

protected abstract PendingTransaction getEvictable();

protected void increaseSpaceUsed(final PendingTransaction pendingTransaction) {
protected void increaseCounters(final PendingTransaction pendingTransaction) {
spaceUsed += pendingTransaction.memorySize();
++txCountByType[pendingTransaction.getTransaction().getType().ordinal()];
}

protected void decreaseSpaceUsed(final PendingTransaction pendingTransaction) {
protected void decreaseCounters(final PendingTransaction pendingTransaction) {
spaceUsed -= pendingTransaction.memorySize();
--txCountByType[pendingTransaction.getTransaction().getType().ordinal()];
}

protected abstract long cacheFreeSpace();
Expand Down
Loading