dex: 🎩 investigate search path duration of 100ms

[cratelyn]

Describe the bug

@hdevalence noticed that there is a conspicuous ceiling of 100ms in the dex component's search path duration metrics, reported in Grafana.

we should investigate this further.

[hdevalence]

It would also be good to get that dashboard into the deployments, I’m not sure how to do that (sent the JSON privately)

[hdevalence]

There are manual bucket configs in the dex metrics module that I don't understand. Could those be part of the cause?

[cratelyn]

There are manual bucket configs in the dex metrics module that I don't understand. Could those be part of the cause?

https://github.com/penumbra-zone/penumbra/blob/main/crates/core/component/dex/src/component/metrics.rs#L49

for posterity, the buckets are defined here.

i would agree that the .1 does appear to be the cause, but i will confirm this hypothesis after the weekend!

[hdevalence]

Do you understand the purpose of those bucket configs? I don’t, so it could be good to document.

[hdevalence]

The surrounding code has a comment that the purpose is to avoid using "Summaries". Why do we want that? Is there a reason to not record all the data and then have the quantiles figured out after the fact?

[cratelyn]

i can't speak to the original rationale, but i dug up this relevant bit from the prometheus docs:

The essential difference between summaries and histograms is that summaries calculate streaming φ-quantiles on the client side and expose them directly, while histograms expose bucketed observation counts and the calculation of quantiles from the buckets of a histogram happens on the server side using the histogram_quantile() function.

source

adding some additional buckets (1 second, 10 seconds) is probably the easiest fix here. a histogram is figuring out the quantiles after the fact, so it seems like the metric type we should stick with.

[erwanor]

Once #4571 lands in main, we will have non-broken DEX buckets so we should be able to close this, defer future performance investigation, and redirect towards closing the loop on other streams like indexing, candlestick data integration/debugging. (cc @aubrika @hdevalence @cratelyn Y/N?)

[hdevalence]

We can close this now, we already identified the cause (manual bucket configs) and fixed the DEX buckets. Further changes to the DEX bucket configs are not necessary based on the information we have now, because we know that we can't get more useful signal out of the GCP testnet deployment, as a result of its virtualized I/O, which prevents us from drawing any conclusions about I/O driven performance. #4571 makes changes based on data we know is bad, so it's not a helpful step towards understanding and improving performance. If we want to do that, we should first make sure that we have good data, collecting measurements on dedicated hardware (cf #4565)

[cratelyn]

#4571 doesn't currently alter bucket configuration. we haven't found consensus about how to configure histograms, but i've opted to drop further changes and stop trying to fix the "ceiling" problem shown in the original image in this issue.

#4524 and #4502 (superseding #4489) changed bucket configuration.

#4571 introduces telemetry measuring the duration of path relaxation and makes changes to allow us to add other metric types like counters, while #4581 adds telemetry to measure scheduler latency. those are changes that build upon @erwanor's fantastic work that suggested that path search accounted for 85% of dex latency.

i'm happy to close this for now as well.