Skip to content

Commit

Permalink
row: mark an error from streamer as StorageError
Browse files Browse the repository at this point in the history
Earlier this year we changed the vectorized panic-catcher, namely, we
now check a few special error types (like `StorageError`,
`notInternalError`, and `internalError` upfront - to avoid more
expensive stack-based checks). If we find an `internalError` _and_ it
doesn't have a PG code and doesn't have `StorageError` or
`notInternalError` in the causes, we now always mark the error as
assertion failure. This made it so that we started receiving more sentry
reports than previously; in particular, whenever we're using the
streamer API, we could now get `stop.ErrUnavailable` via the following
sequence of calls `ColIndexJoin.Next` -> `cFetcher.StartScan` ->
`txnKVStreamer.SetupNextFetch` -> `Streamer.Enqueue`, and this would
trigger a sentry report. This commit fixes this by marking an error
coming from `Streamer.Enqueue` as `StorageError` which will by-pass the
logic in the panic-catcher.

Release note: None
  • Loading branch information
yuzefovich committed Nov 20, 2024
1 parent b31d15e commit dc318bb
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 1 deletion.
1 change: 1 addition & 0 deletions pkg/sql/row/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ go_library(
"//pkg/sql/catalog/fetchpb",
"//pkg/sql/catalog/schemaexpr",
"//pkg/sql/catalog/seqexpr",
"//pkg/sql/colexecerror",
"//pkg/sql/colmem",
"//pkg/sql/isql",
"//pkg/sql/pgwire/pgcode",
Expand Down
9 changes: 8 additions & 1 deletion pkg/sql/row/kv_batch_streamer.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/cockroachdb/cockroach/pkg/kv/kvserver/concurrency/lock"
"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/colexecerror"
"github.com/cockroachdb/cockroach/pkg/sql/rowinfra"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/mon"
Expand Down Expand Up @@ -111,7 +112,13 @@ func (f *txnKVStreamer) SetupNextFetch(
// TODO(yuzefovich): consider supporting COL_BATCH_RESPONSE scan format.
reqs := spansToRequests(spans, kvpb.BATCH_RESPONSE, false /* reverse */, f.rawMVCCValues, f.lockStrength, f.lockDurability, reqsScratch)
if err := f.streamer.Enqueue(ctx, reqs); err != nil {
return err
// Mark this error as having come from the storage layer. This will
// allow us to avoid creating a sentry report since this error isn't
// actionable (e.g. we can get stop.ErrUnavailable here, which would be
// treated as "internal error" by the ColIndexJoin, which later would
// result in treating it as assertion failure because the error doesn't
// have the PG code - marking it as a storage error will skip that).
return colexecerror.NewStorageError(err)
}
// For the spans slice we only need to account for the overhead of
// roachpb.Span objects. This is because spans that correspond to
Expand Down

0 comments on commit dc318bb

Please sign in to comment.