-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Deprecate vttablet metrics QueryCacheXX
and rename to QueryPlanCacheXX
#16289
Changes from 2 commits
9adb889
6cf772d
ecc3210
16a6412
f23888d
623dd6b
c0230d8
ade9210
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 |
---|---|---|
|
@@ -197,26 +197,26 @@ func TestQueryPlanCache(t *testing.T) { | |
ctx := context.Background() | ||
logStats := tabletenv.NewLogStats(ctx, "GetPlanStats") | ||
|
||
initialHits := qe.queryCacheHits.Get() | ||
initialMisses := qe.queryCacheMisses.Get() | ||
Comment on lines
-200
to
-201
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. In order to ensure that we keep testing the old metrics until we delete them, we should keep the tests for them. That will require keeping the struct members also around until the metrics are deleted. Each vttablet has one instance of the queryEngine, so it should not be a big deal. 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. Done via ade9210 |
||
initialHits := qe.queryPlanCacheHits.Get() | ||
initialMisses := qe.queryPlanCacheMisses.Get() | ||
|
||
firstPlan, err := qe.GetPlan(ctx, logStats, firstQuery, false) | ||
require.NoError(t, err) | ||
require.NotNil(t, firstPlan, "plan should not be nil") | ||
|
||
assertPlanCacheSize(t, qe, 1) | ||
|
||
require.Equal(t, int64(0), qe.queryCacheHits.Get()-initialHits) | ||
require.Equal(t, int64(1), qe.queryCacheMisses.Get()-initialMisses) | ||
require.Equal(t, int64(0), qe.queryPlanCacheHits.Get()-initialHits) | ||
require.Equal(t, int64(1), qe.queryPlanCacheMisses.Get()-initialMisses) | ||
|
||
secondPlan, err := qe.GetPlan(ctx, logStats, firstQuery, false) | ||
require.NoError(t, err) | ||
require.NotNil(t, secondPlan, "plan should not be nil") | ||
|
||
assertPlanCacheSize(t, qe, 1) | ||
|
||
require.Equal(t, int64(1), qe.queryCacheHits.Get()-initialHits) | ||
require.Equal(t, int64(1), qe.queryCacheMisses.Get()-initialMisses) | ||
require.Equal(t, int64(1), qe.queryPlanCacheHits.Get()-initialHits) | ||
require.Equal(t, int64(1), qe.queryPlanCacheMisses.Get()-initialMisses) | ||
|
||
qe.ClearQueryPlanCache() | ||
} | ||
|
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.
This flag was removed a while ago, I took the liberty of removing this from the config file.