Conversation
Signed-off-by: Cody Littley <cody.littley@seinetwork.io>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3081 +/- ##
==========================================
+ Coverage 58.33% 58.56% +0.22%
==========================================
Files 2096 2096
Lines 173400 173402 +2
==========================================
+ Hits 101149 101548 +399
+ Misses 63194 62800 -394
+ Partials 9057 9054 -3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
# Conflicts: # sei-db/state_db/bench/cryptosim/cryptosim_config.go # sei-db/state_db/bench/cryptosim/receipt_test.go
| // Deprecated: ignored when LogFilterReadConcurrency > 0. Previously controlled | ||
| // the fraction of shared reads that were log filters vs receipt lookups. | ||
| ReceiptLogFilterRatio float64 |
There was a problem hiding this comment.
If this field is deprecated, why add it to the config?
| // Target total log filter reads per second across all log filter goroutines. | ||
| LogFilterReadsPerSecond int |
There was a problem hiding this comment.
Although I know from context that these new settings are specific to receipt store benchmarking, based on the names of the settings in isolation this fact isn't immediately obvious.
I suggest one or more of the following strategies:
- Adding a line to the godocs saying something like "This is configuration for benchmarking the receipt store, and is ignored if receipt simulation is disabled.", or
- Adding a "Reciept" prefix to the setting name, or
- Nest receipt store configuration in a struct that just contains receipt store configuration (that struct could live in this file or a new file, not strongly opinionated on where we put it)
| } | ||
| } | ||
|
|
||
| func (r *txHashRing) Push(txHash common.Hash, blockNumber uint64, contractAddress common.Address) { |
There was a problem hiding this comment.
Nit, godocs are nice for exported functions
|
|
||
| // RandomEntry returns a random entry from the ring. The caller's rng is used | ||
| // to avoid contention on a shared rng across reader goroutines. | ||
| func (r *txHashRing) RandomEntry(rng *rand.Rand) *txHashEntry { |
There was a problem hiding this comment.
Optional suggestion: consider using canned random as the number generator here. For occasional randomness rand.Rand is fine, but for extremely intense workloads it can sometimes become a benchmark hotsot. Not sure if this will be a hotspot or not, but I generally prefer to default to canned random during benchmarks as a defensive strategy.
// Int64Range returns a random int64 in [min, max). Min is inclusive, max is exclusive.
// If min == max, returns min.
func (cr *CannedRandom) Int64Range(min int64, max int64) int64 {
| // Cryptosim passes its metrics as a read observer so cache hits/misses are measured | ||
| // at the cache wrapper, which is the only layer that can distinguish them reliably. | ||
| store, err := receipt.NewReceiptStoreWithReadObserver(storeCfg, nil, metrics) |
There was a problem hiding this comment.
If it would be useful to be able to intercept this information for the cache, we should discuss potential API. Doesn't sound like a very tricky feature to add.
| // CannedRandom with the same (seed, bufferSize) as the block builder so that | ||
| // SyntheticTxHash produces the same hashes the write path stored. | ||
| // Only SeededBytes is called, which is a read-only operation safe for concurrent use. | ||
| crand := NewCannedRandom(config.CannedRandomSize, config.Seed) |
There was a problem hiding this comment.
Note that this will allocate a 1 gigabyte internal buffer (by default), so this is a very expensive thing to do. Consider using the Clone() method to get a threadsafe instance without duplicating the random number buffer.
// Clone creates a copy of the CannedRandom. On its own, a single CannedRandom is not thread safe. A cloned copy
// is however thread safe, with respect to the original CannedRandom and other cloned copies. Cloning a CannedRandom
// is cheap, and does not require significant additional memory.
//
// If randomizeOffset is true, the clone's read position is set to a new offset derived from the source's buffer,
// ensuring that the clone does not produce random numbers in lockstep with the original. Successive calls to
// Clone(true) yield clones with different offsets.
func (cr *CannedRandom) Clone(randomizeOffset bool) *CannedRandom {
| } | ||
|
|
||
| func selectReceiptReadBlock( | ||
| rng *rand.Rand, |
There was a problem hiding this comment.
Potential hotspot, consider using canned random. (rand.Rand appears in a bunch of places in this file, comment applies to all use sites)
There was a problem hiding this comment.
Note that if there are rand.Rand features you'd like to have that aren't in canned random, it should be fairly straight forward to add additional convenience methods to canned random that return whatever data type is convenient.
3 participants
Describe your changes and provide context
This PR extends the cryptosim receipt benchmark from a write-only workload into a mixed read/write workload that exercises the production receipt store path more realistically. It adds concurrent receipt readers to RecieptStoreSimulator, uses deterministic synthetic tx hashes so readers can reconstruct lookup targets without storing per-tx state, and simulates both receipt-by-hash queries and eth_getLogs-style log filter reads with a configurable recency bias toward newer blocks.
To make that workload measurable, the PR adds receipt read metrics and dashboard panels for read latency, reads/sec, cache hit vs miss rate, found vs not found results, and log filter latency. It also adds an optional receipt read observer so cache hits/misses are recorded at the ledger-cache layer, updates the parquet receipt store to return ErrNotFound cleanly when there is no legacy KV fallback, and refreshes the sample receipt-store config to enable the new benchmark mode. Overall, this gives us a better way to evaluate receipt-store behavior under concurrent reads, writes, cache pressure, and pruning.
Testing performed to validate your change
Ran
go test ./sei-db/ledger_db/receipt/...Ran
go test ./sei-db/state_db/bench/cryptosim/...Added unit coverage for deterministic SyntheticTxHash generation
Added unit coverage for receipt cache hit/miss observer reporting