[Off-chain] feat: in-memory query cache(s) #994

bryanchriswhite · 2024-12-11T15:28:37Z

Summary

Adds the QueryCache[T any] and HistoricalQueryCache[T any] interfaces, InMemoryCache[T any] implementation, configurations, and options functions.

---
title: Legend
---

classDiagram-v2

class GenericInterface__T__any {
    <<interface>>
    GenericMethod() T
}

class Implemenetation {
    ExportedField FieldType
    unexportedField FieldType
}

Implemenetation --|> GenericInterface__T__any: implements

%% class Embedder__T__any {
%%     <<interface>>
%%     GenericInterface[T]
%% }

%% Embedder__T__any ..|> GenericInterface__T__any: embeds

---
title: Query Caches
---

classDiagram-v2


class QueryCache__T__any {
    <<interface>>
    Get(key string) (value T, err error)
    Set(key string, value T) (err error)
    Delete(key string)
    Clear()
}

class HistoricalQueryCache__T__any {
    <<interface>>
    GetLatestVersion(key string) (value T, err error)
    GetVersion(key string, version int64) (value T, err error)
    SetVersion(key string, value T, version int64) (err error)
}

class InMemoryCache__T__any:::cacheImpl
InMemoryCache__T__any --|> QueryCache__T__any
InMemoryCache__T__any --|> HistoricalQueryCache__T__any

Issue

[Off-Chain] ModuleParamsClient & Historical Params #543

Type of change

Select one or more from the following:

New feature, functionality or library
Consensus breaking; add the consensus-breaking label if so. See [Infra] Automatically add the consensus-breaking label #791 for details
Bug fix
Code health or cleanup
Documentation
Other (specify)

Testing

Documentation: make docusaurus_start; only needed if you make doc changes
Unit Tests: make go_develop_and_test
LocalNet E2E Tests: make test_e2e
DevNet E2E Tests: Add the devnet-test-e2e label to the PR.

Sanity Checklist

I have tested my changes using the available tooling
I have commented my code
I have performed a self-review of my own code; both comments & source code
I create and reference any new tickets, if applicable
I have left TODOs throughout the codebase, if applicable

(cherry picked from commit 7d48aef23354497be55958ad2f484e1734550249)

pkg/client/query/cache/memory.go

* pokt/main: [Relayminer, Bug] fix: sessiontree logger never initialized (#993) fix: E2E tests - RPC URL path (#1008) Updated cheat sheat docs with an example after installation (#1004) fix: Nil session tree logger (#1007)

Olshansk

@bryanchriswhite I did a superficial review but did not dive into the validation of the business logic line-by-line.

Is there any section where you'd want another pair of 👀 ?

pkg/client/interface.go

pkg/client/query/cache/config.go

pkg/client/query/cache/memory.go

pkg/client/query/cache/memory_test.go

Olshansk · 2024-12-14T02:32:17Z

pkg/client/query/cache/memory.go

+	c.itemsMu.Lock()
+	defer c.itemsMu.Unlock()
+
+	delete(c.items, key)


What if we delete the (key,value) pair for latestHeight?

Could you elaborate? I'm not quite following.

Yea, this comment doesn't make sense if you don't know where I was looking or what I was thinking...

If we delete the key for the latest height, do we need to also make an appropriate call to c.latestHeight.Store(??)?

If this is indeed the case, please add a unit test for it.

If not, would just appreciate a quick (no need for anything big) explanation.

Gotcha, good question. I don't think we need to update c.latestHeight, regardless of which key is being deleted. For Gets, this should be completely transparent to the caller; for Sets, it means that if you call Delete, followed by Set (as opposed to SetAtHeight), then the height you're setting will be the last c.latestHeight. I think this behavior is correct.

If not, would just appreciate a quick (no need for anything big) explanation.

I did see 👆 but had to think it through for myself anyways:

flowchart LR subgraph hc2[Historical Cache] lh2["latestHeight: 2"] subgraph k2_2h["key2 history"] k2_2h1["height: 1; value 100"] end subgraph k2_1h["key1 history"] k2_1h1["height: 1; value 10"] k2_1h2["height: 2; value 20"] end end subgraph hc3[Historical Cache] lh3["latestHeight: 2"] subgraph k3_2h["key2 history"] k3_2h1["height: 1; value 100"] end end subgraph hc4[Historical Cache] lh4["latestHeight: 2"] subgraph k4_1h["key1 history"] k4_1h2["height: 2; value 30"] end end hc2 --"Delete(key1)"--> hc3 hc3 --"Set(key1, 30)"--> hc4 lh2 --> k2_1h2 lh4 --> k4_1h2

Loading

Given the explanation and diagram, I don't fully understand the need for latestHeight at all. In fact, I think we should delete it now if the behaviour you described is expected.

I failed to reconcile with this thread but after discussing with @red-0ne, we decided that it should be an error to call Set() on a HistoricalQueryCache. I think this indicates that HistoricalQueryCache SHOULD NOT embed QueryCache after all. Subsequently, latestVersion is no longer necessary and can be removed.

// QueryCache is a key/value store style interface for a cache of a single type. // It is intended to be used to cache query responses (or derivatives thereof), // where each key uniquely indexes the most recent query response. type QueryCache[T any] interface { Get(key string) (T, error) Set(key string, value T) error Delete(key string) Clear() } // HistoricalQueryCache extends QueryCache to support getting and setting values // at multiple heights for a given key. type HistoricalQueryCache[T any] interface { // GetLatestVersion retrieves the historical value with the highest version number. GetLatestVersion(key string) (T, error) // GetAsOfVersion retrieves the nearest value <= the specified version number. GetAsOfVersion(key string, version int64) (T, error) // SetAsOfVersion adds or updates a value at a specific version number. SetAsOfVersion(key string, value T, version int64) error }

(class diagrams also updated)

pkg/client/query/cache/memory_test.go

Co-authored-by: Daniel Olshansky <[email protected]>

pkg/client/query/cache/memory.go

Olshansk

Love the iteration. Thanks @bryanchriswhite!

Left a few new comments, a few replies to older threads, but it should be g2g after the next round 🙌

pkg/client/query/cache/config.go

pkg/client/query/cache/memory.go

Olshansk · 2024-12-17T01:28:07Z

pkg/client/query/cache/memory.go

+}
+
+// GetAtHeight retrieves the value from the cache with the given key, at the given
+// height. If a value is not found for that height, the value at the nearest previous


When I see a function like GetAtHeight, my immediate thought is that it'll return the value at the height provided (if available) or nil otherwise.

For height=5, it's the difference between select * from table where height=5 and select * from table where height <= 5.

There could be business logic that depends on state transitions AT A SPECIFIC HEIGHT, so I'm just afraid that it's not explicit enough.

pkg/client/query/cache/memory.go

Olshansk · 2024-12-17T01:37:44Z

pkg/client/query/cache/memory.go

+	c.itemsMu.Lock()
+	defer c.itemsMu.Unlock()
+
+	delete(c.items, key)


Yea, this comment doesn't make sense if you don't know where I was looking or what I was thinking...

If we delete the key for the latest height, do we need to also make an appropriate call to c.latestHeight.Store(??)?

If this is indeed the case, please add a unit test for it.

If not, would just appreciate a quick (no need for anything big) explanation.

pkg/client/query/cache/memory.go

red-0ne

Great caching design.

Left some comments but did not tend to pkg/client/query/cache/memory_test.go yet. Which I'll do right after this one.

pkg/client/query/cache/config.go

pkg/client/query/cache/memory.go

red-0ne

Asking for a small change, but otherwise LGTM.

Not approving yet as I want review the tests too.

pkg/client/query/cache/memory.go

Olshansk · 2024-12-20T00:53:01Z

pkg/client/interface.go

-	// SetAtHeight adds or updates a value at a specific height
-	SetAtHeight(key string, value T, height int64) error
+	// GetAsOfVersion retrieves the nearest value <= the specified version number.
+	GetAsOfVersion(key string, version int64) (T, error)


Can we go with one of the following instead:

GetVersioned

GetAtVersion

GetHistorical

Don't care which one but AsOf in a function name just doesn't feel right.

In that case, GetVersion and SetVersion would be my preference. Any objections @red-0ne @Olshansk?

Also adding GetLatestVersion.

pkg/client/query/cache/config.go

Olshansk · 2024-12-20T00:56:44Z

pkg/client/query/cache/config.go

-	// values. If 0, no historical pruning is performed. It only applies when
-	// historical is true.
-	pruneOlderThan int64
+	// maxVersionAge is the max difference between the latest known version and


this sentence is hard to grok.

Any chance you could improve it with an example?

Olshansk · 2024-12-20T01:00:06Z

pkg/client/query/cache/memory.go

-	items map[string]any
+type inMemoryCache[T any] struct {
+	config        queryCacheConfig
+	latestVersion atomic.Int64


Per my other comment, can you #PUC why we need this and how its used?

bryanchriswhite added 4 commits December 11, 2024 16:21

fix: sessiontree bug

830a7f6

(cherry picked from commit 7d48aef23354497be55958ad2f484e1734550249)

chore: add QueryCache interface

b0f08c3

chore: add HistoricalQueryCache interface

8743246

feat: add InMemoryCache implementation

d895441

bryanchriswhite added the off-chain Off-chain business logic label Dec 11, 2024

bryanchriswhite added this to the Shannon Beta TestNet Support milestone Dec 11, 2024

bryanchriswhite self-assigned this Dec 11, 2024

bryanchriswhite linked an issue Dec 11, 2024 that may be closed by this pull request

[Off-Chain] ModuleParamsClient & Historical Params #543

Open

4 tasks

bryanchriswhite commented Dec 11, 2024

View reviewed changes

pkg/client/query/cache/memory.go Outdated Show resolved Hide resolved

bryanchriswhite added 3 commits December 12, 2024 10:52

chore: self-review improvements

23cc94a

chore: self-review improvements

366ab1d

fix: historical pruning

4632c74

bryanchriswhite marked this pull request as ready for review December 12, 2024 11:20

bryanchriswhite requested review from red-0ne and Olshansk December 12, 2024 11:20

bryanchriswhite changed the base branch from fix/sessiontree to main December 13, 2024 14:24

Olshansk requested changes Dec 14, 2024

View reviewed changes

chore: review feedback improvements

e48a2f2

Co-authored-by: Daniel Olshansky <[email protected]>

github-actions bot reviewed Dec 16, 2024

View reviewed changes

pkg/client/query/cache/memory.go Outdated Show resolved Hide resolved

bryanchriswhite force-pushed the issues/543/cache/memory branch from 4ac0eef to 011d2d7 Compare December 16, 2024 12:27

chore: review feedback improvements

d5ce62f

bryanchriswhite force-pushed the issues/543/cache/memory branch from 011d2d7 to d5ce62f Compare December 16, 2024 12:29

bryanchriswhite requested a review from Olshansk December 16, 2024 12:30

Olshansk requested changes Dec 17, 2024

View reviewed changes

red-0ne reviewed Dec 18, 2024

View reviewed changes

bryanchriswhite added 4 commits December 18, 2024 13:09

chore: review feedback improvements

71da7f1

fix: linter errors

6774d3a

chore: disable #Set() on historical cache

086d219

Merge branch 'main' into issues/543/cache/memory

d6dbc44

bryanchriswhite requested review from Olshansk and red-0ne December 18, 2024 12:56

red-0ne reviewed Dec 18, 2024

View reviewed changes

pkg/client/query/cache/memory.go Show resolved Hide resolved

pkg/client/query/cache/memory.go Show resolved Hide resolved

bryanchriswhite requested a review from red-0ne December 19, 2024 08:02

Merge branch 'main' into issues/543/cache/memory

0414e9a

Olshansk requested changes Dec 20, 2024

View reviewed changes

bryanchriswhite mentioned this pull request Dec 20, 2024

[Testing, Tooling] Expose integration app via gRPC/HTTP/WS #1017

Draft

15 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Off-chain] feat: in-memory query cache(s) #994

[Off-chain] feat: in-memory query cache(s) #994

bryanchriswhite commented Dec 11, 2024 •

edited

Loading

Olshansk left a comment

Olshansk Dec 14, 2024

bryanchriswhite Dec 16, 2024 •

edited

Loading

Olshansk Dec 17, 2024

bryanchriswhite Dec 18, 2024 •

edited

Loading

Olshansk Dec 20, 2024

bryanchriswhite Dec 20, 2024 •

edited

Loading

bryanchriswhite Dec 20, 2024 •

edited

Loading

Olshansk left a comment

Olshansk Dec 17, 2024

Olshansk Dec 17, 2024

red-0ne left a comment

red-0ne left a comment

Olshansk Dec 20, 2024

bryanchriswhite Dec 20, 2024 •

edited

Loading

bryanchriswhite Dec 20, 2024

Olshansk Dec 20, 2024

Olshansk Dec 20, 2024

bryanchriswhite Dec 20, 2024

[Off-chain] feat: in-memory query cache(s) #994

Are you sure you want to change the base?

[Off-chain] feat: in-memory query cache(s) #994

Conversation

bryanchriswhite commented Dec 11, 2024 • edited Loading

Summary

Issue

Type of change

Testing

Sanity Checklist

Olshansk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanchriswhite Dec 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanchriswhite Dec 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanchriswhite Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

bryanchriswhite Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Olshansk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

red-0ne left a comment

Choose a reason for hiding this comment

red-0ne left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanchriswhite Dec 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bryanchriswhite commented Dec 11, 2024 •

edited

Loading

bryanchriswhite Dec 16, 2024 •

edited

Loading

bryanchriswhite Dec 18, 2024 •

edited

Loading

bryanchriswhite Dec 20, 2024 •

edited

Loading

bryanchriswhite Dec 20, 2024 •

edited

Loading

bryanchriswhite Dec 20, 2024 •

edited

Loading