-
Notifications
You must be signed in to change notification settings - Fork 3.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
61583: sql: issue GetRequests from SQL when possible r=jordanlewis a=jordanlewis Closes #46758. Previously, SQL unconditionally emitted ScanRequests for every read that it produced, even if we were completely sure that the scan could contain only a single key that was fully known at request time. This was less efficient than necessary, because ScanRequests cause the KV layer to do more work than GetRequests. Here are the reasons why GetRequests are more efficient: - MVCCGet, unlike MVCCScan, is able to use a prefix iterator, which allows it to make use of RocksDB/Pebble bloom filters. There may also be other wins at the storage layer, like avoiding prefetching - various data structures have optimizations and fast-paths for single-key operations (e.g. timestamp cache, latch manager, refresh span tracking) - various code paths are simpler in the point operation case - a point operation implies only needing a single key allocation instead of a start and end key allocation - a point operation implies only needing to return up to a single result, which means that we can avoid some indirection in various places - A scan over a span that has at most 1 key, but does not know it, needs to iterate over all the versions (or eventually do a second expensive seek), looking for the non-existent next key in the span. A Get can do a single seek, read the key, and be done. Release note (performance improvement): SQL will now emit GetRequests when possible to KV instead of always emitting ScanRequests. This manifests as a modest performance improvement for some workloads. Co-authored-by: Jordan Lewis <[email protected]>
- Loading branch information
Showing
41 changed files
with
594 additions
and
295 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,96 @@ | ||
// Copyright 2021 The Cockroach Authors. | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
package batcheval | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
"github.com/cockroachdb/cockroach/pkg/roachpb" | ||
"github.com/cockroachdb/cockroach/pkg/storage" | ||
"github.com/cockroachdb/cockroach/pkg/util/leaktest" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
// TestGetResumeSpan tests that a GetRequest with a target bytes or max span | ||
// request keys is properly handled by returning no result with a resume span. | ||
func TestGetResumeSpan(t *testing.T) { | ||
defer leaktest.AfterTest(t)() | ||
ctx := context.Background() | ||
resp := &roachpb.GetResponse{} | ||
key := roachpb.Key([]byte{'a'}) | ||
value := roachpb.MakeValueFromString("woohoo") | ||
|
||
db := storage.NewDefaultInMemForTesting() | ||
defer db.Close() | ||
|
||
_, err := Put(ctx, db, CommandArgs{ | ||
Header: roachpb.Header{TargetBytes: -1}, | ||
Args: &roachpb.PutRequest{ | ||
RequestHeader: roachpb.RequestHeader{ | ||
Key: key, | ||
}, | ||
Value: value, | ||
}, | ||
}, resp) | ||
assert.NoError(t, err) | ||
|
||
// Case 1: Check that a negative TargetBytes causes a resume span. | ||
_, err = Get(ctx, db, CommandArgs{ | ||
Header: roachpb.Header{TargetBytes: -1}, | ||
Args: &roachpb.GetRequest{ | ||
RequestHeader: roachpb.RequestHeader{ | ||
Key: key, | ||
}, | ||
}, | ||
}, resp) | ||
assert.NoError(t, err) | ||
|
||
assert.NotNil(t, resp.ResumeSpan) | ||
assert.Equal(t, key, resp.ResumeSpan.Key) | ||
assert.Nil(t, resp.ResumeSpan.EndKey) | ||
assert.Nil(t, resp.Value) | ||
|
||
resp = &roachpb.GetResponse{} | ||
// Case 2: Check that a negative MaxSpanRequestKeys causes a resume span. | ||
_, err = Get(ctx, db, CommandArgs{ | ||
Header: roachpb.Header{MaxSpanRequestKeys: -1}, | ||
Args: &roachpb.GetRequest{ | ||
RequestHeader: roachpb.RequestHeader{ | ||
Key: key, | ||
}, | ||
}, | ||
}, resp) | ||
assert.NoError(t, err) | ||
|
||
assert.NotNil(t, resp.ResumeSpan) | ||
assert.Equal(t, key, resp.ResumeSpan.Key) | ||
assert.Nil(t, resp.ResumeSpan.EndKey) | ||
assert.Nil(t, resp.Value) | ||
|
||
resp = &roachpb.GetResponse{} | ||
// Case 3: Check that a positive limit causes a normal return. | ||
_, err = Get(ctx, db, CommandArgs{ | ||
Header: roachpb.Header{MaxSpanRequestKeys: 10, TargetBytes: 100}, | ||
Args: &roachpb.GetRequest{ | ||
RequestHeader: roachpb.RequestHeader{ | ||
Key: key, | ||
}, | ||
}, | ||
}, resp) | ||
assert.NoError(t, err) | ||
|
||
assert.Nil(t, resp.ResumeSpan) | ||
assert.NotNil(t, resp.Value) | ||
assert.Equal(t, resp.Value.RawBytes, value.RawBytes) | ||
assert.Equal(t, 1, int(resp.NumKeys)) | ||
assert.Equal(t, len(resp.Value.RawBytes), int(resp.NumBytes)) | ||
} |
Oops, something went wrong.