-
Notifications
You must be signed in to change notification settings - Fork 12
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
[Off-Chain, Scalability] Query client caching & history #985
Conversation
pkg/client/query/sharedquerier.go
Outdated
); err != nil { | ||
return nil, err | ||
} | ||
|
||
querier.sharedQuerier = sharedtypes.NewQueryClient(querier.clientConn) | ||
sq.paramsCache = cache.NewInMemoryCache[*sharedtypes.Params]( | ||
// TODO_IN_THIS_COMMIT: extract to constants. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract to constants.
pkg/client/query/sharedquerier.go
Outdated
sq.paramsCache = cache.NewInMemoryCache[*sharedtypes.Params]( | ||
// TODO_IN_THIS_COMMIT: extract to constants. | ||
cache.WithHistoricalMode(100), | ||
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxSize doesn't apply to historical mode... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxSize doesn't apply to historical mode...
pkg/client/query/sharedquerier.go
Outdated
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxSize doesn't apply to historical mode... | ||
//cache.WithMaxSize(1), | ||
cache.WithEvictionPolicy(cache.FirstInFirstOut), | ||
// TODO_IN_THIS_COMMIT: extract to a constant. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract to a constant.
pkg/client/query/cache/config.go
Outdated
// EvictionPolicy determines how items are removed when cache is full | ||
type EvictionPolicy string | ||
|
||
// TODO_IN_THIS_COMMIT: refactor to an enum. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: refactor to an enum.
pkg/client/query/cache/config.go
Outdated
// historical is whether the cache will cache a single value for each key | ||
// (false) or whether it will cache a history of values for each key (true). | ||
historical bool | ||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
pkg/client/query/sessionquerier.go
Outdated
|
||
// Initialize params cache with minimal configuration since we only need latest | ||
sq.paramsCache = cache.NewInMemoryCache[*sessiontypes.Params]( | ||
// TODO_IN_THIS_COMMIT: extract to a constant. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract to a constant.
pkg/client/query/sessionquerier.go
Outdated
sq.paramsCache = cache.NewInMemoryCache[*sessiontypes.Params]( | ||
// TODO_IN_THIS_COMMIT: extract to a constant. | ||
cache.WithHistoricalMode(100), | ||
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxSize doesn't apply to historical mode... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxSize doesn't apply to historical mode...
pkg/client/query/sessionquerier.go
Outdated
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxSize doesn't apply to historical mode... | ||
//cache.WithMaxSize(1), | ||
cache.WithEvictionPolicy(cache.FirstInFirstOut), | ||
// TODO_IN_THIS_COMMIT: extract to a constant. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract to a constant.
pkg/client/query/sessionquerier.go
Outdated
); err != nil { | ||
return nil, err | ||
} | ||
|
||
sessq.sessionQuerier = sessiontypes.NewQueryClient(sessq.clientConn) | ||
// TODO_IN_THIS_COMMIT: kick off a goroutine that subscribes to params updates and populates the cache. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: kick off a goroutine that subscribes to params updates and populates the cache.
@@ -187,6 +187,12 @@ func (b *blockReplayClient) queryLatestBlock( | |||
errCh := make(chan error) | |||
|
|||
go func() { | |||
// TODO_IN_THIS_COMMIT: extract labels (and values?) to constants. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract labels (and values?) to constants.
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.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
linter-name (fail-on-found)
x/shared/types/query_client.go|22 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/application/types/query_client.go|9 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/application/types/query_client.go|16 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/application/types/query_client.go|21 col 4| // TODO_IN_THIS_COMMIT: investigate generalization...
x/application/types/query_client.go|22 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/service/types/query_client.go|9 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/service/types/query_client.go|16 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/service/types/query_client.go|21 col 4| // TODO_IN_THIS_COMMIT: investigate generalization...
x/service/types/query_client.go|22 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/session/types/query_client.go|9 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/session/types/query_client.go|16 col 4| // TODO_IN_THIS_COMMIT: godoc...
x/session/types/query_client.go|21 col 4| // TODO_IN_THIS_COMMIT: investigate generalization...
x/session/types/query_client.go|22 col 4| // TODO_IN_THIS_COMMIT: godoc...
prooftypes "github.com/pokt-network/poktroll/x/proof/types" | ||
) | ||
|
||
// TODO_IN_THIS_COMMIT: comment explaining why we can't use client.ProofQueryClient; |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: comment explaining why we can't use client.ProofQueryClient;
func NewProofQuerier( | ||
deps depinject.Config, | ||
paramsQuerierOpts ...ParamsQuerierOptionFn, | ||
// TODO_IN_THIS_COMMIT: comment explaining why we can't use client.ProofQueryClient; |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: comment explaining why we can't use client.ProofQueryClient;
pkg/client/query/proofquerier.go
Outdated
} | ||
|
||
querier := &proofQuerier{ | ||
// TODO_IN_THIS_COMMIT: extract this to an option. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract this to an option.
pkg/client/query/sharedquerier.go
Outdated
} | ||
|
||
sq := &sharedQuerier{ | ||
// TODO_IN_THIS_COMMIT: extract this to an option. |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: extract this to an option.
pkg/client/query/paramsquerier.go
Outdated
"github.com/pokt-network/poktroll/pkg/polylog" | ||
) | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
return NewQueryClient(conn).(ProofQueryClient) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: investigate generalization...
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... | ||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
gogogrpc "github.com/cosmos/gogoproto/grpc" | ||
) | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
GetParams(context.Context) (*Params, error) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
return NewQueryClient(conn).(SharedQueryClient) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: investigate generalization...
gogogrpc "github.com/cosmos/gogoproto/grpc" | ||
) | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
GetParams(context.Context) (*Params, error) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
return NewQueryClient(conn).(SupplierQueryClient) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: investigate generalization...
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... | ||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... | ||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... | ||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
gogogrpc "github.com/cosmos/gogoproto/grpc" | ||
) | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
GetParams(context.Context) (*Params, error) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
return NewQueryClient(conn).(SessionQueryClient) | ||
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: investigate generalization...
} | ||
|
||
// TODO_IN_THIS_COMMIT: investigate generalization... | ||
// TODO_IN_THIS_COMMIT: godoc... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: godoc...
@@ -12,6 +12,8 @@ go 1.23.0 | |||
// github.com/pokt-network/smt/kvstore/pebble => ../smt/kvstore/pebble | |||
// ) | |||
|
|||
replace nhooyr.io/websocket => github.com/coder/websocket v1.8.6 |
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.
revert
go.sum
Outdated
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.
revert
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.
revert
pkg/client/query/options.go
Outdated
CacheOpts: []cache.CacheOption{ | ||
// TODO_IN_THIS_COMMIT: extract to constants. | ||
cache.WithHistoricalMode(100), | ||
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxItems doesn't apply to historical mode... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxItems doesn't apply to historical mode...
@@ -51,6 +72,8 @@ func (servq *serviceQuerier) GetService( | |||
Id: serviceId, | |||
} | |||
|
|||
// TODO_IN_THIS_COMMIT: historically cache services... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: historically cache services...
@@ -71,6 +94,8 @@ func (servq *serviceQuerier) GetServiceRelayDifficulty( | |||
ServiceId: serviceId, | |||
} | |||
|
|||
// TODO_IN_THIS_COMMIT: historically cache relay mining difficulties... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: historically cache relay mining difficulties...
CacheOpts: []cache.CacheOption{ | ||
// TODO_IN_THIS_COMMIT: extract to constants. | ||
cache.WithHistoricalMode(100), | ||
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxKeys doesn't apply to historical mode... |
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.
[linter-name (fail-on-found)] reported by reviewdog 🐶
// TODO_IN_THIS_COMMIT: reconcile the fact that MaxKeys doesn't apply to historical mode...
6f5f1c3
to
b07a8b8
Compare
Summary
Adds caching to the query clients.
Issue
Type of change
Select one or more from the following:
consensus-breaking
label if so. See [Infra] Automatically add theconsensus-breaking
label #791 for detailsTesting
make docusaurus_start
; only needed if you make doc changesmake go_develop_and_test
make test_e2e
devnet-test-e2e
label to the PR.Sanity Checklist