Make profitability histogram metric buckets configurable

Signed-off-by: Fabio Di Fabio <[email protected]>
Consensys · Nov 4, 2024 · fcb4001 · fcb4001
1 parent 1564c77
commit fcb4001
Show file tree

Hide file tree

Showing 10 changed files with 176 additions and 90 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1,5 +1,5 @@
 releaseVersion=0.8.0-rc4.1
-besuVersion=24.10-local
+besuVersion=24.11-delivery37
 arithmetizationVersion=0.8.0-rc4
 besuArtifactGroup=io.consensys.linea-besu
 distributionIdentifier=linea-sequencer

diff --git a/sequencer/src/main/java/net/consensys/linea/config/LineaProfitabilityCliOptions.java b/sequencer/src/main/java/net/consensys/linea/config/LineaProfitabilityCliOptions.java
@@ -57,6 +57,12 @@ public class LineaProfitabilityCliOptions implements LineaCliOptions {
       "--plugin-linea-extra-data-set-min-gas-price-enabled";
   public static final boolean DEFAULT_EXTRA_DATA_SET_MIN_GAS_PRICE_ENABLED = true;
 
+  public static final String PROFITABILITY_METRICS_BUCKETS =
+      "--plugin-linea-profitability-metrics-buckets";
+  public static final double[] DEFAULT_PROFITABILITY_METRICS_BUCKETS = {
+    0.9, 1.0, 1.2, 2, 5, 10, 100, 1000
+  };
+
   @Positive
   @CommandLine.Option(
       names = {FIXED_GAS_COST_WEI},
@@ -135,6 +141,16 @@ public class LineaProfitabilityCliOptions implements LineaCliOptions {
           "Enable setting min gas price runtime value via extra data field (default: ${DEFAULT-VALUE})")
   private boolean extraDataSetMinGasPriceEnabled = DEFAULT_EXTRA_DATA_SET_MIN_GAS_PRICE_ENABLED;
 
+  @CommandLine.Option(
+      names = {PROFITABILITY_METRICS_BUCKETS},
+      arity = "1..*",
+      split = ",",
+      hidden = true,
+      paramLabel = "<FLOAT[]>",
+      description =
+          "List of buckets to use to create the histogram for profitability metrics (default: ${DEFAULT-VALUE})")
+  private double[] profitabilityMetricsBuckets = DEFAULT_PROFITABILITY_METRICS_BUCKETS;
+
   private LineaProfitabilityCliOptions() {}
 
   /**
@@ -164,6 +180,7 @@ public static LineaProfitabilityCliOptions fromConfig(
     options.txPoolCheckP2pEnabled = config.txPoolCheckP2pEnabled();
     options.extraDataPricingEnabled = config.extraDataPricingEnabled();
     options.extraDataSetMinGasPriceEnabled = config.extraDataSetMinGasPriceEnabled();
+    options.profitabilityMetricsBuckets = config.profitabilityMetricsBuckets();
     return options;
   }
 
@@ -184,6 +201,7 @@ public LineaProfitabilityConfiguration toDomainObject() {
         .txPoolCheckP2pEnabled(txPoolCheckP2pEnabled)
         .extraDataPricingEnabled(extraDataPricingEnabled)
         .extraDataSetMinGasPriceEnabled(extraDataSetMinGasPriceEnabled)
+        .profitabilityMetricsBuckets(profitabilityMetricsBuckets)
         .build();
   }
 
@@ -199,6 +217,7 @@ public String toString() {
         .add(TX_POOL_ENABLE_CHECK_P2P, txPoolCheckP2pEnabled)
         .add(EXTRA_DATA_PRICING_ENABLED, extraDataPricingEnabled)
         .add(EXTRA_DATA_SET_MIN_GAS_PRICE_ENABLED, extraDataSetMinGasPriceEnabled)
+        .add(PROFITABILITY_METRICS_BUCKETS, profitabilityMetricsBuckets)
         .toString();
   }
 }
diff --git a/sequencer/src/main/java/net/consensys/linea/config/LineaProfitabilityConfiguration.java b/sequencer/src/main/java/net/consensys/linea/config/LineaProfitabilityConfiguration.java
@@ -33,6 +33,7 @@ public class LineaProfitabilityConfiguration implements LineaOptionsConfiguratio
   /** It is safe to keep this as long, since it will store value <= max_int * 1000 */
   private long variableCostWei;
 
+  /** It is safe to keep this as long, since it will store value <= max_int * 1000 */
   private long ethGasPriceWei;
 
   private double minMargin;
@@ -42,6 +43,7 @@ public class LineaProfitabilityConfiguration implements LineaOptionsConfiguratio
   private boolean txPoolCheckP2pEnabled;
   private boolean extraDataPricingEnabled;
   private boolean extraDataSetMinGasPriceEnabled;
+  private double[] profitabilityMetricsBuckets;
 
   /**
    * These 2 parameters must be atomically updated

diff --git a/sequencer/src/main/java/net/consensys/linea/metrics/HistogramMetrics.java b/sequencer/src/main/java/net/consensys/linea/metrics/HistogramMetrics.java
@@ -22,6 +22,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.function.DoubleSupplier;
 
 import lombok.extern.slf4j.Slf4j;
 import org.apache.commons.lang3.StringUtils;
@@ -32,23 +33,22 @@
 
 @Slf4j
 public class HistogramMetrics {
-
   public interface LabelValue {
     String value();
   }
 
-  private static final double[] DEFAULT_HISTOGRAM_BUCKETS = {0.9, 1.0, 1.2, 2, 5, 10, 100, 1000};
   private static final String LABEL_VALUES_SEPARATOR = "\u2060";
   private final LabelledMetric<Histogram> histogram;
-  private final Map<String, Double> mins;
-  private final Map<String, Double> maxs;
+  private final Map<String, MutableDoubleSupplier> mins;
+  private final Map<String, MutableDoubleSupplier> maxs;
 
   @SafeVarargs
   public HistogramMetrics(
       final MetricsSystem metricsSystem,
       final LineaMetricCategory category,
       final String name,
       final String help,
+      final double[] buckets,
       final Class<? extends LabelValue>... labels) {
 
     final var labelNames = getLabelNames(labels);
@@ -64,19 +64,17 @@ public HistogramMetrics(
     maxs = HashMap.newHashMap(combinations.size());
     for (final var combination : combinations) {
       final var key = String.join(LABEL_VALUES_SEPARATOR, combination);
-      mins.put(key, Double.POSITIVE_INFINITY);
-      minRatio.labels(() -> mins.get(key), combination);
-      maxs.put(key, 0.0);
-      maxRatio.labels(() -> maxs.get(key), combination);
+      final var minSupplier = new MutableDoubleSupplier(Double.POSITIVE_INFINITY);
+      mins.put(key, minSupplier);
+      minRatio.labels(minSupplier, combination);
+      final var maxSupplier = new MutableDoubleSupplier(Double.NEGATIVE_INFINITY);
+      maxs.put(key, maxSupplier);
+      maxRatio.labels(maxSupplier, combination);
     }
 
     this.histogram =
         metricsSystem.createLabelledHistogram(
-            category,
-            name,
-            StringUtils.capitalize(help) + " buckets",
-            DEFAULT_HISTOGRAM_BUCKETS,
-            labelNames);
+            category, name, StringUtils.capitalize(help) + " buckets", buckets, labelNames);
   }
 
   @SafeVarargs
@@ -115,15 +113,40 @@ private List<String[]> getLabelValuesCombinations(final Class<? extends LabelVal
 
   public void track(final double value, final String... labelValues) {
 
+    // Record the observation
+    histogram.labels(labelValues).observe(value);
+  }
+
+  public void setMinMax(final double min, final double max, final String... labelValues) {
     final var key = String.join(LABEL_VALUES_SEPARATOR, labelValues);
 
     // Update lowest seen
-    mins.compute(key, (unused, currMin) -> Math.min(currMin, value));
+    mins.get(key).set(min);
 
     // Update highest seen
-    maxs.compute(key, (unused, currMax) -> Math.max(currMax, value));
+    maxs.get(key).set(max);
+  }
 
-    // Record the observation
-    histogram.labels(labelValues).observe(value);
+  private static class MutableDoubleSupplier implements DoubleSupplier {
+    private final double initialValue;
+    private volatile double value;
+
+    public MutableDoubleSupplier(final double initialValue) {
+      this.initialValue = initialValue;
+      this.value = initialValue;
+    }
+
+    @Override
+    public double getAsDouble() {
+      return value;
+    }
+
+    public void set(final double value) {
+      this.value = value;
+    }
+
+    public void reset() {
+      value = initialValue;
+    }
   }
 }
diff --git a/...nsensys/linea/sequencer/txpoolvalidation/metrics/TransactionPoolProfitabilityMetrics.java b/...nsensys/linea/sequencer/txpoolvalidation/metrics/TransactionPoolProfitabilityMetrics.java
@@ -16,6 +16,8 @@
 
 import static net.consensys.linea.metrics.LineaMetricCategory.TX_POOL_PROFITABILITY;
 
+import java.util.stream.Collectors;
+
 import lombok.extern.slf4j.Slf4j;
 import net.consensys.linea.bl.TransactionProfitabilityCalculator;
 import net.consensys.linea.config.LineaProfitabilityConfiguration;
@@ -60,23 +62,40 @@ public TransactionPoolProfitabilityMetrics(
     this.blockchainService = blockchainService;
     this.histogramMetrics =
         new HistogramMetrics(
-            metricsSystem, TX_POOL_PROFITABILITY, "ratio", "transaction pool profitability ratio");
+            metricsSystem,
+            TX_POOL_PROFITABILITY,
+            "ratio",
+            "transaction pool profitability ratio",
+            profitabilityConf.profitabilityMetricsBuckets());
   }
 
   public void update() {
     final long startTime = System.currentTimeMillis();
     final var txPoolContent = transactionPoolService.getPendingTransactions();
-    txPoolContent.parallelStream()
-        .map(PendingTransaction::getTransaction)
-        .forEach(this::handleTransaction);
+
+    final var ratioStats =
+        txPoolContent.parallelStream()
+            .map(PendingTransaction::getTransaction)
+            .map(
+                tx -> {
+                  final var ratio = handleTransaction(tx);
+                  histogramMetrics.track(ratio);
+                  log.trace("Recorded profitability ratio {} for tx {}", ratio, tx.getHash());
+                  return ratio;
+                })
+            .collect(Collectors.summarizingDouble(Double::doubleValue));
+
+    histogramMetrics.setMinMax(ratioStats.getMin(), ratioStats.getMax());
+
     log.atDebug()
-        .setMessage("Transaction pool profitability metrics processed {}txs in {}ms")
+        .setMessage("Transaction pool profitability metrics processed {}txs in {}ms, statistics {}")
         .addArgument(txPoolContent::size)
         .addArgument(() -> System.currentTimeMillis() - startTime)
+        .addArgument(ratioStats)
         .log();
   }
 
-  private void handleTransaction(final Transaction transaction) {
+  private double handleTransaction(final Transaction transaction) {
     final Wei actualPriorityFeePerGas;
     if (transaction.getMaxPriorityFeePerGas().isEmpty()) {
       actualPriorityFeePerGas =
@@ -102,8 +121,6 @@ private void handleTransaction(final Transaction transaction) {
         actualPriorityFeePerGas.toBigInteger().doubleValue()
             / profitablePriorityFeePerGas.toBigInteger().doubleValue();
 
-    histogramMetrics.track(ratio);
-
-    log.trace("Recorded profitability ratio {} for tx {}", ratio, transaction.getHash());
+    return ratio;
   }
 }
diff --git a/.../main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectorFactory.java b/.../main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectorFactory.java
@@ -22,11 +22,10 @@
 import net.consensys.linea.config.LineaTracerConfiguration;
 import net.consensys.linea.config.LineaTransactionSelectorConfiguration;
 import net.consensys.linea.jsonrpc.JsonRpcManager;
+import net.consensys.linea.metrics.HistogramMetrics;
 import net.consensys.linea.plugins.config.LineaL1L2BridgeSharedConfiguration;
 import net.consensys.linea.sequencer.txselection.selectors.LineaTransactionSelector;
 import org.hyperledger.besu.plugin.services.BlockchainService;
-import org.hyperledger.besu.plugin.services.MetricsSystem;
-import org.hyperledger.besu.plugin.services.metrics.MetricCategoryRegistry;
 import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelector;
 import org.hyperledger.besu.plugin.services.txselection.PluginTransactionSelectorFactory;
 
@@ -41,8 +40,7 @@ public class LineaTransactionSelectorFactory implements PluginTransactionSelecto
   private final LineaL1L2BridgeSharedConfiguration l1L2BridgeConfiguration;
   private final LineaProfitabilityConfiguration profitabilityConfiguration;
   private final LineaTracerConfiguration tracerConfiguration;
-  private final MetricsSystem metricsSystem;
-  private final MetricCategoryRegistry metricCategoryRegistry;
+  private final Optional<HistogramMetrics> maybeProfitabilityMetrics;
 
   private final Map<String, Integer> limitsMap;
 
@@ -54,17 +52,15 @@ public LineaTransactionSelectorFactory(
       final LineaTracerConfiguration tracerConfiguration,
       final Map<String, Integer> limitsMap,
       final Optional<JsonRpcManager> rejectedTxJsonRpcManager,
-      final MetricsSystem metricsSystem,
-      final MetricCategoryRegistry metricCategoryRegistry) {
+      final Optional<HistogramMetrics> maybeProfitabilityMetrics) {
     this.blockchainService = blockchainService;
     this.txSelectorConfiguration = txSelectorConfiguration;
     this.l1L2BridgeConfiguration = l1L2BridgeConfiguration;
     this.profitabilityConfiguration = profitabilityConfiguration;
     this.tracerConfiguration = tracerConfiguration;
     this.limitsMap = limitsMap;
     this.rejectedTxJsonRpcManager = rejectedTxJsonRpcManager;
-    this.metricsSystem = metricsSystem;
-    this.metricCategoryRegistry = metricCategoryRegistry;
+    this.maybeProfitabilityMetrics = maybeProfitabilityMetrics;
   }
 
   @Override
@@ -77,7 +73,6 @@ public PluginTransactionSelector create() {
         tracerConfiguration,
         limitsMap,
         rejectedTxJsonRpcManager,
-        metricsSystem,
-        metricCategoryRegistry);
+        maybeProfitabilityMetrics);
   }
 }
diff --git a/...c/main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectorPlugin.java b/...c/main/java/net/consensys/linea/sequencer/txselection/LineaTransactionSelectorPlugin.java
@@ -26,6 +26,8 @@
 import net.consensys.linea.config.LineaRejectedTxReportingConfiguration;
 import net.consensys.linea.config.LineaTransactionSelectorConfiguration;
 import net.consensys.linea.jsonrpc.JsonRpcManager;
+import net.consensys.linea.metrics.HistogramMetrics;
+import net.consensys.linea.sequencer.txselection.selectors.ProfitableTransactionSelector;
 import org.hyperledger.besu.plugin.BesuContext;
 import org.hyperledger.besu.plugin.BesuPlugin;
 import org.hyperledger.besu.plugin.services.BesuConfiguration;
@@ -85,6 +87,18 @@ public void start() {
                             lineaRejectedTxReportingConfiguration)
                         .start());
 
+    final Optional<HistogramMetrics> maybeProfitabilityMetrics =
+        metricCategoryRegistry.isMetricCategoryEnabled(SEQUENCER_PROFITABILITY)
+            ? Optional.of(
+                new HistogramMetrics(
+                    metricsSystem,
+                    SEQUENCER_PROFITABILITY,
+                    "ratio",
+                    "sequencer profitability ratio",
+                    profitabilityConfiguration().profitabilityMetricsBuckets(),
+                    ProfitableTransactionSelector.Phase.class))
+            : Optional.empty();
+
     transactionSelectionService.registerPluginTransactionSelectorFactory(
         new LineaTransactionSelectorFactory(
             blockchainService,
@@ -94,8 +108,7 @@ public void start() {
             tracerConfiguration(),
             createLimitModules(tracerConfiguration()),
             rejectedTxJsonRpcManager,
-            metricsSystem,
-            metricCategoryRegistry));
+            maybeProfitabilityMetrics));
   }
 
   @Override