-
Notifications
You must be signed in to change notification settings - Fork 885
Part 2: Bigtable node read fallback #3437
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.
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
base: mvcc-bigtable-pt1
Are you sure you want to change the base?
Changes from all commits
e04092d
32b55d2
6c6f589
4fcd242
0fa1aef
5ef70ba
37d0086
4a14131
f11f0e8
d3035ec
9890f78
66f8c9f
ecdf5cc
35f7143
429ee45
a406321
1a9736a
571a943
b3e0b81
4e37dd6
eea4a85
c564e9e
292f501
e2b358f
2435876
3b0a3e6
87a9d5f
9a4a366
d40b29f
af6f3f9
c3eac8d
024c3a8
ab50f39
8d4044f
3a2d4d8
d44d816
284d1c1
9fc34be
2fa06ee
44c2b26
cbfb450
66b3d0e
db3f1bd
57a67ac
8cb49c1
6cc315f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -219,6 +219,17 @@ | |
| return s.cosmosStore.GetEarliestVersion() | ||
| } | ||
|
|
||
| // GetEarliestVersionForKey reports the earliest version still held locally by | ||
| // the store that serves storeKey. Cosmos and EVM prune independently (an EVM | ||
| // prune failure is only logged), so pruned-read fallback must consult the | ||
| // horizon of the store that will actually serve the read. | ||
| func (s *CompositeStateStore) GetEarliestVersionForKey(storeKey string) int64 { | ||
| if s.evmRouted(storeKey) { | ||
| return s.evmStore.GetEarliestVersion() | ||
| } | ||
| return s.cosmosStore.GetEarliestVersion() | ||
| } | ||
|
Check failure on line 231 in sei-db/state_db/ss/composite/store.go
|
||
|
Comment on lines
+222
to
+231
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 GetEarliestVersionForKey("evm") delegates to EVMStateStore.GetEarliestVersion(), which under evm-ss-separate-dbs takes the MINIMUM earliest version across the 5 independently-pruned EVM sub-DBs — the least-pruned (most optimistic) value, not a safe lower bound. If one sub-DB lags behind the others (e.g. a transient prune failure, since EVMStateStore.Prune only logs errors and never retries), FallbackStateStore.shouldFallback will see the lagging sub-DB's earliest and skip the Bigtable fallback for a read that actually needs to be routed to a different, more-pruned sub-DB — that sub-DB's own MVCC self-gate then silently returns (nil, nil) even though the data exists in Bigtable. Extended reasoning...The bug. func (s *EVMStateStore) GetEarliestVersion() int64 {
var minVersion int64 = -1
for _, db := range s.managedDBs {
if v := db.GetEarliestVersion(); minVersion < 0 || v < minVersion {
minVersion = v
}
}
...
}Why MIN is wrong here. When The failure path. Step-by-step proof. (1) A read arrives for a Why nothing catches this today. Before this PR, Fix. Either change Scope/severity note. This requires stacking two non-default, internal/Giga-only features ( |
||
|
|
||
| func (s *CompositeStateStore) Close() error { | ||
| s.closeOnce.Do(func() { | ||
| if s.pruningManager != nil { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package historical | ||
|
|
||
| import ( | ||
| "context" | ||
|
|
||
| "go.opentelemetry.io/otel" | ||
| "go.opentelemetry.io/otel/attribute" | ||
| "go.opentelemetry.io/otel/metric" | ||
| ) | ||
|
|
||
| // fallbackMeterName is the OpenTelemetry meter for the node-side pruned-read | ||
| // fallback path, exported through the same process-wide MeterProvider. | ||
| const fallbackMeterName = "seidb_historical_fallback" | ||
|
|
||
| const ( | ||
| fallbackOutcomeCacheHit = "cache_hit" | ||
| fallbackOutcomeBackendHit = "backend_hit" | ||
| fallbackOutcomeBackendMiss = "backend_miss" | ||
| fallbackOutcomeBackendBehind = "backend_behind" | ||
| fallbackOutcomeError = "error" | ||
| ) | ||
|
|
||
| // fallbackMetrics counts pruned point reads by operation (get/has) and outcome | ||
| // so operators can see how much load the read cache absorbs and how many reads | ||
| // actually reach the historical backend. Attributes are a closed set, so | ||
| // cardinality stays flat. | ||
| type fallbackMetrics struct { | ||
| reads metric.Int64Counter | ||
| } | ||
|
|
||
| func newFallbackMetrics() *fallbackMetrics { | ||
| meter := otel.Meter(fallbackMeterName) | ||
| reads, _ := meter.Int64Counter( | ||
| "historical_fallback_reads_total", | ||
| metric.WithDescription("Pruned point reads routed to the historical fallback, by operation and outcome"), | ||
| metric.WithUnit("{read}"), | ||
| ) | ||
| return &fallbackMetrics{reads: reads} | ||
| } | ||
|
|
||
| func (m *fallbackMetrics) recordRead(op, outcome string) { | ||
| if m == nil { | ||
| return | ||
| } | ||
| m.reads.Add(context.Background(), 1, metric.WithAttributes( | ||
| attribute.String("op", op), | ||
| attribute.String("outcome", outcome), | ||
| )) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package historical | ||
|
|
||
| import "testing" | ||
|
|
||
| func TestFallbackMetricsNilSafe(t *testing.T) { | ||
| var m *fallbackMetrics | ||
| m.recordRead("get", fallbackOutcomeCacheHit) | ||
| } | ||
|
|
||
| func TestFallbackMetricsRecordNoPanic(t *testing.T) { | ||
| m := newFallbackMetrics() | ||
| for _, outcome := range []string{ | ||
| fallbackOutcomeCacheHit, | ||
| fallbackOutcomeBackendHit, | ||
| fallbackOutcomeBackendMiss, | ||
| fallbackOutcomeError, | ||
| } { | ||
| m.recordRead("get", outcome) | ||
| m.recordRead("has", outcome) | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: