Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
66795: sql: added missing tables from mysql information_schema r=rafiss a=mnovelodou

Previously, there was missing tables on information_schema present on
mysql
This was inadequate because it can cause compatibility problems
To address this, this patch adds missing information_schema tables

Release note (sql change): Added missing tables from mysql
information_schema. The tables are not populated and are
entirely empty.

- column_statistics
- columns_extensions
- engines
- events
- files
- keywords
- optimizer_trace
- partitions
- plugins
- processlist
- profiling
- resource_groups
- schemata_extensions
- st_geometry_columns
- st_spatial_reference_systems
- st_units_of_measure
- table_constraints_extensions
- tables_extensions
- tablespaces
- tablespaces_extensions
- user_attributes

67326: rowexec: remove unnecessary joiner member r=andreimatei a=andreimatei

The joiner had a field tracking a confusing lookup row index. Besides
being confusing, this member obscured the fact that it was equal to the
ordinal returned by inserting rows into a disk container - and thus that
the respective ordinal is used.

Release note: None

67514: sql,kv: permit txn rollbacks across LockNotAvailable errors r=nvanbenschoten a=nvanbenschoten

This commit adds support for rolling a transaction back across a `LockNotAvailable` (pgcode 55P03) error. `LockNotAvailable` errors are returned in two cases:
1. when a locking SELECT is run with a NOWAIT wait policy and conflicts with an active lock
2. when a statement is run with a `lock_timeout` and this timeout is exceeded (unsupported, see #67513)

The following test case from `pkg/sql/testdata/savepoints` demonstrates this new capability:

```
# txn1
BEGIN
INSERT INTO t VALUES (1)
----
1: BEGIN -- 0 rows
-- NoTxn       -> Open        #.  (none)
2: INSERT INTO t VALUES (1) -- 1 row
-- Open        -> Open        ##  (none)

# txn2
BEGIN
SAVEPOINT foo
SELECT * FROM t WHERE x = 1 FOR UPDATE NOWAIT
ROLLBACK TO SAVEPOINT foo
SELECT * FROM t WHERE x = 2 FOR UPDATE NOWAIT
COMMIT
----
1: BEGIN -- 0 rows
-- NoTxn       -> Open        #.....  (none)
2: SAVEPOINT foo -- 0 rows
-- Open        -> Open        ##....  foo
3: SELECT * FROM t WHERE x = 1 FOR UPDATE NOWAIT -- pq: could not obtain lock on row (x)=(1) in t@primary
-- Open        -> Aborted     XXXXXX  foo
4: ROLLBACK TO SAVEPOINT foo -- 0 rows
-- Aborted     -> Open        ##....  foo
5: SELECT * FROM t WHERE x = 2 FOR UPDATE NOWAIT -- 0 rows
-- Open        -> Open        ##..#.  foo
6: COMMIT -- 0 rows
-- Open        -> NoTxn       ##..##  (none)
```

This becomes the second error type that supports rollbacks, with the first being duplicate key errors, which was added in 65e8045.

The primary motivation for this PR was to be able to give `WriteIntentErrors` an `ErrorPriority` of `ErrorScoreUnambiguousError` for #66146. However, the added functionality fell out of making that change.

Release note (sql change): ROLLBACK TO SAVEPOINT can now be used to recover from LockNotAvailable errors (pgcode 55P03), which are returned when performing a FOR UPDATE SELECT with a NOWAIT wait policy.

67524: limit,storage: add more trace spans to backup path r=dt a=adityamaru

This change adds a trace recording to track how
many requests are waiting in the the concurrent limiter
queue.

The change also adds a child span to ExportMVCCToSst
to track how long the scan+SST creation is taking per
ExportRequest during backup.

Release note: None

67544: catalogkv: fix panic inside Txn swallowing retries r=postamar a=ajwerner

`(*kv.DB).Txn` may produce retry errors. While it's fine that this test
function can panic in the face of a real error, it's not okay to panic
the retry error.

Fixes #67213.

Release note: None

67550: authors: add xinhaoz to AUTHORS r=xinhaoz a=xinhaoz

Release note: None

67557: parser: reduce imports for scan.go r=otan a=rafiss

See individual commits:
- move keywords.go and tokens.go to lexbase 
- remove scan.go dependency on tree NewNumVal/NewStrVal

Touches #64710

Co-authored-by: MiguelNovelo <[email protected]>
Co-authored-by: Andrei Matei <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Aditya Maru <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>
Co-authored-by: Xin Hao Zhang <[email protected]>
Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
8 people committed Jul 14, 2021
8 parents 774c000 + e0cdc5e + 748c79e + 1325712 + fe57f71 + 763d236 + e6201bd + 209c5f0 commit 5a0b11e
Show file tree
Hide file tree
Showing 56 changed files with 2,752 additions and 1,191 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ Wade Waldron <[email protected]>
wenyong-h <[email protected]>
Will Haack <[email protected]> <[email protected]>
Xiang Li <[email protected]>
Xin Hao Zhang <[email protected]> <[email protected]>
Xinyu Zhou (Joe) <[email protected]>
XisiHuang <[email protected]>
xphoniex <[email protected]>
Expand Down
4 changes: 2 additions & 2 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ load("@bazel_gazelle//:def.bzl", "gazelle")
# gazelle:exclude pkg/sql/parser/sql.go
# gazelle:exclude pkg/sql/parser/helpmap_test.go
# gazelle:exclude pkg/sql/parser/help_messages.go
# gazelle:exclude pkg/sql/lex/keywords.go
# gazelle:exclude pkg/sql/lex/tokens.go
# gazelle:exclude pkg/sql/lexbase/keywords.go
# gazelle:exclude pkg/sql/lexbase/tokens.go
# gazelle:exclude pkg/sql/lexbase/reserved_keywords.go
# gazelle:exclude pkg/cmd/prereqs/testdata
# gazelle:exclude pkg/testutils/**/testdata/**
Expand Down
14 changes: 7 additions & 7 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -811,8 +811,8 @@ SQLPARSER_TARGETS = \
pkg/sql/parser/sql.go \
pkg/sql/parser/helpmap_test.go \
pkg/sql/parser/help_messages.go \
pkg/sql/lex/tokens.go \
pkg/sql/lex/keywords.go \
pkg/sql/lexbase/tokens.go \
pkg/sql/lexbase/keywords.go \
pkg/sql/lexbase/reserved_keywords.go

WKTPARSER_TARGETS = \
Expand Down Expand Up @@ -1458,11 +1458,11 @@ pkg/sql/parser/gen/sql.go.tmp: pkg/sql/parser/gen/sql-gen.y bin/.bootstrap
# functions and lexing predicates need to know about keywords, and
# keywords map to the token constants. Therefore, generate the
# constant tokens in the lex package primarily.
pkg/sql/lex/tokens.go: pkg/sql/parser/gen/sql.go.tmp
pkg/sql/lexbase/tokens.go: pkg/sql/parser/gen/sql.go.tmp
(echo "// Code generated by make. DO NOT EDIT."; \
echo "// GENERATED FILE DO NOT EDIT"; \
echo; \
echo "package lex"; \
echo "package lexbase"; \
echo; \
grep '^const [A-Z][_A-Z0-9]* ' $^) > $@.tmp || rm $@.tmp
mv -f $@.tmp $@
Expand All @@ -1474,7 +1474,7 @@ pkg/sql/parser/sql.go: pkg/sql/parser/gen/sql.go.tmp | bin/.bootstrap
(echo "// Code generated by goyacc. DO NOT EDIT."; \
echo "// GENERATED FILE DO NOT EDIT"; \
cat $^ | \
sed -E 's/^const ([A-Z][_A-Z0-9]*) =.*$$/const \1 = lex.\1/g') > $@.tmp || rm $@.tmp
sed -E 's/^const ([A-Z][_A-Z0-9]*) =.*$$/const \1 = lexbase.\1/g') > $@.tmp || rm $@.tmp
mv -f $@.tmp $@
goimports -w $@

Expand Down Expand Up @@ -1512,8 +1512,8 @@ pkg/sql/lexbase/reserved_keywords.go: pkg/sql/parser/sql.y pkg/sql/parser/reserv
mv -f $@.tmp $@
gofmt -s -w $@

pkg/sql/lex/keywords.go: pkg/sql/parser/sql.y pkg/sql/lex/allkeywords/main.go | bin/.bootstrap
$(GO) run $(GOMODVENDORFLAGS) -tags all-keywords pkg/sql/lex/allkeywords/main.go < $< > $@.tmp || rm $@.tmp
pkg/sql/lexbase/keywords.go: pkg/sql/parser/sql.y pkg/sql/lexbase/allkeywords/main.go | bin/.bootstrap
$(GO) run $(GOMODVENDORFLAGS) -tags all-keywords pkg/sql/lexbase/allkeywords/main.go < $< > $@.tmp || rm $@.tmp
mv -f $@.tmp $@
gofmt -s -w $@

Expand Down
2 changes: 1 addition & 1 deletion pkg/ccl/storageccl/export.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ func evalExport(
var curSizeOfExportedSSTs int64
for start := args.Key; start != nil; {
destFile := &storage.MemFile{}
summary, resume, err := reader.ExportMVCCToSst(start, args.EndKey, args.StartTime,
summary, resume, err := reader.ExportMVCCToSst(ctx, start, args.EndKey, args.StartTime,
h.Timestamp, exportAllRevisions, targetSize, maxSize, useTBI, destFile)
if err != nil {
return result.Result{}, err
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/storageccl/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,7 @@ func assertEqualKVs(
maxSize := uint64(0)
prevStart := start
sstFile := &storage.MemFile{}
summary, start, err = e.ExportMVCCToSst(start, endKey, startTime, endTime,
summary, start, err = e.ExportMVCCToSst(ctx, start, endKey, startTime, endTime,
exportAllRevisions, targetSize, maxSize, enableTimeBoundIteratorOptimization, sstFile)
require.NoError(t, err)
sst = sstFile.Data()
Expand Down Expand Up @@ -609,7 +609,7 @@ func assertEqualKVs(
if dataSizeWhenExceeded == maxSize {
maxSize--
}
_, _, err = e.ExportMVCCToSst(prevStart, endKey, startTime, endTime,
_, _, err = e.ExportMVCCToSst(ctx, prevStart, endKey, startTime, endTime,
exportAllRevisions, targetSize, maxSize, enableTimeBoundIteratorOptimization, &storage.MemFile{})
require.Regexp(t, fmt.Sprintf("export size \\(%d bytes\\) exceeds max size \\(%d bytes\\)",
dataSizeWhenExceeded, maxSize), err)
Expand Down
4 changes: 2 additions & 2 deletions pkg/cmd/github-post/testdata/stress-unknown.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@
{"Action":"output","Output":"awk -f ./pkg/sql/parser/replace_help_rules.awk \u003e pkg/sql/parser/gen/sql.y\n"}
{"Action":"output","Output":"awk -f ./pkg/sql/parser/reserved_keywords.awk \u003c pkg/sql/parser/sql.y \u003e pkg/sql/lexbase/reserved_keywords.go.tmp || rm pkg/sql/lexbase/reserved_keywords.go.tmp\n"}
{"Action":"output","Output":"mv -f pkg/sql/parser/help_messages.go.tmp pkg/sql/parser/help_messages.go\n"}
{"Action":"output","Output":"mv -f pkg/sql/lex/keywords.go.tmp pkg/sql/lex/keywords.go\n"}
{"Action":"output","Output":"mv -f pkg/sql/lexbase/keywords.go.tmp pkg/sql/lexbase/keywords.go\n"}
{"Action":"output","Output":"mv -f pkg/sql/lexbase/reserved_keywords.go.tmp pkg/sql/lexbase/reserved_keywords.go\n"}
{"Action":"output","Output":"gofmt -s -w pkg/sql/parser/help_messages.go\n"}
{"Action":"output","Output":"gofmt -s -w pkg/sql/lex/keywords.go\n"}
{"Action":"output","Output":"gofmt -s -w pkg/sql/lexbase/keywords.go\n"}
{"Action":"output","Output":"gofmt -s -w pkg/sql/lexbase/reserved_keywords.go\n"}
{"Action":"output","Output":"mv -f pkg/sql/parser/helpmap_test.go.tmp pkg/sql/parser/helpmap_test.go\n"}
{"Action":"output","Output":"gofmt -s -w pkg/sql/parser/helpmap_test.go\n"}
Expand Down
17 changes: 17 additions & 0 deletions pkg/kv/kvclient/kvcoord/dist_sender_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3324,6 +3324,7 @@ func TestMultipleErrorsMerged(t *testing.T) {
retryErr := roachpb.NewTransactionRetryError(roachpb.RETRY_SERIALIZABLE, "test err")
abortErr := roachpb.NewTransactionAbortedError(roachpb.ABORT_REASON_ABORTED_RECORD_FOUND)
conditionFailedErr := &roachpb.ConditionFailedError{}
writeIntentErr := &roachpb.WriteIntentError{}
sendErr := sendError{}
ambiguousErr := &roachpb.AmbiguousResultError{}
randomErr := &roachpb.IntegerOverflowError{}
Expand Down Expand Up @@ -3394,6 +3395,22 @@ func TestMultipleErrorsMerged(t *testing.T) {
err2: randomErr,
expErr: "results in overflow",
},
// WriteIntentError also has a low score since it's "not ambiguous".
{
err1: writeIntentErr,
err2: ambiguousErr,
expErr: "result is ambiguous",
},
{
err1: writeIntentErr,
err2: sendErr,
expErr: "failed to send RPC",
},
{
err1: writeIntentErr,
err2: randomErr,
expErr: "results in overflow",
},
}
for i, tc := range testCases {
t.Run(strconv.Itoa(i), func(t *testing.T) {
Expand Down
41 changes: 41 additions & 0 deletions pkg/kv/kvclient/kvcoord/testdata/savepoints
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,47 @@ subtest end



subtest rollback_after_write_intent_error
# Write intent errors are white-listed to allow a rollback to savepoint afterwards.
# They make their way back up to the kv client when requests are run with an Error
# wait policy.

# NB: we're going to leak this txn, so write to an otherwise unused key.
begin
----
0 <noignore>

put conflict-key a
----

begin
----
0 <noignore>

savepoint x
----
0 <noignore>

get conflict-key locking nowait
----
(*roachpb.WriteIntentError) conflicting intents on "conflict-key"

rollback x
----
0 <noignore>

put conflict-key b nowait
----
(*roachpb.WriteIntentError) conflicting intents on "conflict-key"

rollback x
----
1 [1-1]

subtest end



subtest rollback_after_random_err
# Only CPut errors allow rollbacks after them. Any other error results in the rollback failing.

Expand Down
14 changes: 7 additions & 7 deletions pkg/kv/kvclient/kvcoord/txn_coord_sender_savepoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,13 @@ func (tc *TxnCoordSender) RollbackToSavepoint(ctx context.Context, s kv.Savepoin
}

// We don't allow rollback to savepoint after errors (except after
// ConditionFailedError which is special-cased elsewhere and doesn't move the
// txn to the txnError state). In particular, we cannot allow rollbacks to
// savepoint after ambiguous errors where it's possible for a
// previously-successfully written intent to have been pushed at a timestamp
// higher than the coordinator's WriteTimestamp. Doing so runs the risk that
// we'll commit at the lower timestamp, at which point the respective intent
// will be discarded. See
// ConditionFailedError and WriteIntentError, which are special-cased
// elsewhere and don't move the txn to the txnError state). In particular, we
// cannot allow rollbacks to savepoint after ambiguous errors where it's
// possible for a previously-successfully written intent to have been pushed
// at a timestamp higher than the coordinator's WriteTimestamp. Doing so runs
// the risk that we'll commit at the lower timestamp, at which point the
// respective intent will be discarded. See
// https://github.com/cockroachdb/cockroach/issues/47587.
//
// TODO(andrei): White-list more errors.
Expand Down
28 changes: 21 additions & 7 deletions pkg/kv/kvclient/kvcoord/txn_coord_sender_savepoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/kv"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvserverbase"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/testutils/kvclientutils"
Expand Down Expand Up @@ -122,9 +123,12 @@ func TestSavepoints(t *testing.T) {
fmt.Fprintf(&buf, "txn id %s\n", changed)

case "put":
if err := txn.Put(ctx,
roachpb.Key(td.CmdArgs[0].Key),
[]byte(td.CmdArgs[1].Key)); err != nil {
b := txn.NewBatch()
b.Put(td.CmdArgs[0].Key, td.CmdArgs[1].Key)
if td.HasArg("nowait") {
b.Header.WaitPolicy = lock.WaitPolicy_Error
}
if err := txn.Run(ctx, b); err != nil {
fmt.Fprintf(&buf, "(%T) %v\n", err, err)
}

Expand All @@ -150,12 +154,22 @@ func TestSavepoints(t *testing.T) {
}

case "get":
v, err := txn.Get(ctx, td.CmdArgs[0].Key)
if err != nil {
b := txn.NewBatch()
if td.HasArg("locking") {
b.GetForUpdate(td.CmdArgs[0].Key)
} else {
b.Get(td.CmdArgs[0].Key)
}
if td.HasArg("nowait") {
b.Header.WaitPolicy = lock.WaitPolicy_Error
}
if err := txn.Run(ctx, b); err != nil {
fmt.Fprintf(&buf, "(%T) %v\n", err, err)
} else {
ba, _ := v.Value.GetBytes()
fmt.Fprintf(&buf, "%v -> %v\n", v.Key, string(ba))
kv := b.Results[0].Rows[0]
ba, err := kv.Value.GetBytes()
require.NoError(t, err)
fmt.Fprintf(&buf, "%v -> %v\n", kv.Key, string(ba))
}

case "savepoint":
Expand Down
25 changes: 25 additions & 0 deletions pkg/kv/kvclient/kvcoord/txn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -725,6 +725,31 @@ func TestTxnContinueAfterCputError(t *testing.T) {
require.NoError(t, txn.Commit(ctx))
}

// Test that a transaction can be used after a locking request returns a
// WriteIntentError. This is not generally allowed for other errors, but
// WriteIntentError is special.
func TestTxnContinueAfterWriteIntentError(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
ctx := context.Background()
s := createTestDB(t)
defer s.Stop()

otherTxn := s.DB.NewTxn(ctx, "lock holder txn")
require.NoError(t, otherTxn.Put(ctx, "a", "b"))

txn := s.DB.NewTxn(ctx, "test txn")

b := txn.NewBatch()
b.Header.WaitPolicy = lock.WaitPolicy_Error
b.Put("a", "c")
err := txn.Run(ctx, b)
require.IsType(t, &roachpb.WriteIntentError{}, err)

require.NoError(t, txn.Put(ctx, "a'", "c"))
require.NoError(t, txn.Commit(ctx))
}

func TestTxnWaitPolicies(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down
4 changes: 4 additions & 0 deletions pkg/kv/kvnemesis/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,8 @@
// - Root and leaf transactions
// - GCRequest
// - Protected timestamps
// - Transactions being abandoned by their coordinator
// - Continuing txns after CPut and WriteIntent errors (generally continuing
// after errors is not allowed, but it is allowed after ConditionFailedError and
// WriteIntentError as a special case)
package kvnemesis
6 changes: 0 additions & 6 deletions pkg/kv/kvnemesis/kvnemesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,6 @@ import (

// RunNemesis generates and applies a series of Operations to exercise the KV
// api. It returns a slice of the logical failures encountered.
//
// Ideas for conditions to be added to KV nemesis:
// - Transactions being abandoned by their coordinator.
// - CPuts, and continuing after CPut errors (generally continuing after errors
// is not allowed, but it is allowed after ConditionFailedError as a special
// case).
func RunNemesis(
ctx context.Context,
rng *rand.Rand,
Expand Down
3 changes: 2 additions & 1 deletion pkg/kv/kvserver/spanset/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -399,14 +399,15 @@ func (s spanSetReader) Closed() bool {

// ExportMVCCToSst is part of the storage.Reader interface.
func (s spanSetReader) ExportMVCCToSst(
ctx context.Context,
startKey, endKey roachpb.Key,
startTS, endTS hlc.Timestamp,
exportAllRevisions bool,
targetSize, maxSize uint64,
useTBI bool,
dest io.Writer,
) (roachpb.BulkOpSummary, roachpb.Key, error) {
return s.r.ExportMVCCToSst(startKey, endKey, startTS, endTS, exportAllRevisions, targetSize,
return s.r.ExportMVCCToSst(ctx, startKey, endKey, startTS, endTS, exportAllRevisions, targetSize,
maxSize, useTBI, dest)
}

Expand Down
16 changes: 9 additions & 7 deletions pkg/roachpb/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ const (
// written. We allow the transaction to continue after such errors; we also
// allow RollbackToSavepoint() to be called after such errors. In particular,
// this is useful for SQL which wants to allow rolling back to a savepoint
// after ConditionFailedErrors (uniqueness violations). With continuing after
// errors its important for the coordinator to track the timestamp at which
// intents might have been written.
// after ConditionFailedErrors (uniqueness violations) and WriteIntentError
// (lock not available errors). With continuing after errors its important for
// the coordinator to track the timestamp at which intents might have been
// written.
//
// Note that all the lower scores also are unambiguous in this sense, so this
// score can be seen as an upper-bound for unambiguous errors.
Expand Down Expand Up @@ -132,11 +133,12 @@ func ErrPriority(err error) ErrorPriority {
return ErrorScoreTxnAbort
}
return ErrorScoreTxnRestart
case *ConditionFailedError:
case *ConditionFailedError, *WriteIntentError:
// We particularly care about returning the low ErrorScoreUnambiguousError
// because we don't want to transition a transaction that encounters
// ConditionFailedError to an error state. More specifically, we want to
// allow rollbacks to savepoint after a ConditionFailedError.
// because we don't want to transition a transaction that encounters a
// ConditionFailedError or a WriteIntentError to an error state. More
// specifically, we want to allow rollbacks to savepoint after one of these
// errors.
return ErrorScoreUnambiguousError
}
return ErrorScoreNonRetriable
Expand Down
Loading

0 comments on commit 5a0b11e

Please sign in to comment.