Skip to content
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

sqlbase: link to issue #42508 in sequence-related errors #42509

Merged
merged 1 commit into from
Nov 18, 2019

Conversation

knz
Copy link
Contributor

@knz knz commented Nov 15, 2019

(I intend to back-port this to 19.2 and perhaps 19.1.)

Informs #42508.
Informs #42510.

This patch fixes two things.

  1. When the backfiller encounters a sequence operation it fails with a
    hard error. This and consequences is described further in issue sql: cannot add a new sequence-populated column (or pg-compatible SERIAL) to an existing table, + bug #42508.
    To inform the user better, this patch links the error message to that issue.
    Additionally, this also ensures that telemetry usage is reported
    for the unimplemented feature.

For example:

[email protected]:39861/movr> alter table t add column y int default nextval('s');
pq: nextval(): unimplemented: cannot backfill such sequence operation
HINT: You have attempted to use a feature that is not yet implemented.
See: https://github.com/cockroachdb/cockroach/issues/42508

and

> begin;
alter table t add column y int default nextval('s');
commit;
pq: transaction committed but schema change aborted with error: (0A000): nextval(): unimplemented: cannot backfill such sequence operation
HINT: You have attempted to use a feature that is not yet implemented.
See: https://github.com/cockroachdb/cockroach/issues/42508
--
Some of the non-DDL statements may have committed successfully, but some of the DDL statement(s) failed.
Manual inspection may be required to determine the actual state of the database.
--
See: https://github.com/cockroachdb/cockroach/issues/42061
  1. The other thing. Prior to this patch, an error "under" a backfill
    would be flattened into error code 42P15 (invalid schema), even if
    the schema was valid and the error was really about something
    encountered during the backfill. This patch fixes it. For example,
 ALTER TABLE shopping ADD COLUMN c int AS (quantity::int // 0) STORED

Would previously error out with code 42P15, whereas the true error is
22012 (division by zero) which is reported now.

Release note (sql change): CockroachDB will now properly refer to
issue #42508 in the error message hint when a client attempts
to add a sequence-based column to an existing table, which
is an unimplemented feature.

Release note (sql change): CockroachDB will now report an more
accurate error message, hint and error code if/when an error is
encountered while adding a new column.

@knz knz requested a review from thoszhang November 15, 2019 09:56
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@thoszhang thoszhang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 4 of 4 files at r1.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@knz
Copy link
Contributor Author

knz commented Nov 17, 2019

cool thanks!

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 17, 2019

Build failed

@knz
Copy link
Contributor Author

knz commented Nov 17, 2019

transient agent failure - retrying

bors r+

@craig
Copy link
Contributor

craig bot commented Nov 17, 2019

Build failed

@knz
Copy link
Contributor Author

knz commented Nov 18, 2019

Ok I'm not going to blame the CI agents further and instead investigate this. Later this week.

@knz
Copy link
Contributor Author

knz commented Nov 18, 2019

Ok I went to CI and I see the servers failing to shut down because they are stuck trying to send stuff to our telemetry servers.

Example hanging goroutine:

goroutine 832 [select]:
net/http.(*persistConn).roundTrip(0xc003789680, 0xc003777ef0, 0x0, 0x0, 0x0)
	/usr/local/go/src/net/http/transport.go:2194 +0x75c
net/http.(*Transport).roundTrip(0x7100300, 0xc0037e9e00, 0x8630aa, 0xc00376e248, 0xc0035bf4d0)
	/usr/local/go/src/net/http/transport.go:481 +0xa1b
net/http.(*Transport).RoundTrip(0x7100300, 0xc0037e9e00, 0x7100300, 0x0, 0x0)
	/usr/local/go/src/net/http/roundtrip.go:17 +0x35
net/http.send(0xc0037e9e00, 0x4a02360, 0x7100300, 0x0, 0x0, 0x0, 0xc0010faf70, 0xc0035bf6f8, 0x1, 0x0)
	/usr/local/go/src/net/http/client.go:250 +0x461
net/http.(*Client).send(0x75339a0, 0xc0037e9e00, 0x0, 0x0, 0x0, 0xc0010faf70, 0x0, 0x1, 0xc00048c380)
	/usr/local/go/src/net/http/client.go:174 +0xfb
net/http.(*Client).do(0x75339a0, 0xc0037e9e00, 0x0, 0x0, 0x0)
	/usr/local/go/src/net/http/client.go:641 +0x279
net/http.(*Client).Do(...)
	/usr/local/go/src/net/http/client.go:509
net/http.(*Client).Get(0x75339a0, 0xc00376e240, 0x108, 0xc0006e9800, 0x1, 0x0)
	/usr/local/go/src/net/http/client.go:398 +0x9e
net/http.Get(...)
	/usr/local/go/src/net/http/client.go:370
github.com/cockroachdb/cockroach/pkg/server.(*Server).checkForUpdates(0xc0006e9800, 0x4a75e40, 0xc0008b41b0, 0x15ca41e7, 0xed5648200)
	/go/src/github.com/cockroachdb/cockroach/pkg/server/updates.go:234 +0x19d
github.com/cockroachdb/cockroach/pkg/server.(*Server).maybeCheckForUpdates(0xc0006e9800, 0x4a75e40, 0xc0008b41b0, 0xc2ce802, 0xed56482e8, 0x0, 0x31fd701b, 0xed56482e7, 0x0, 0x15ca41e7, ...)
	/go/src/github.com/cockroachdb/cockroach/pkg/server/updates.go:152 +0xdf
github.com/cockroachdb/cockroach/pkg/server.(*Server).PeriodicallyCheckForUpdates.func1(0x4a75e40, 0xc0008b41b0)
	/go/src/github.com/cockroachdb/cockroach/pkg/server/updates.go:116 +0x273
github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunWorker.func1(0xc000399fc0, 0xc000760a20, 0xc000399fb0)
	/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:197 +0xfb
created by github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunWorker
	/go/src/github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:190 +0xa8

@knz
Copy link
Contributor Author

knz commented Nov 18, 2019

CI will go better after #42537

This patch fixes two things.

1) When the backfiller encounters a sequence operation it fails with a
hard error. This and consequences is described further in issue cockroachdb#42508.
To inform the user better, this patch links the error message to that issue.
Additionally, this also ensures that telemetry usage is reported
for the unimplemented feature.

For example:

```
[email protected]:39861/movr> alter table t add column y int default nextval('s');
pq: nextval(): unimplemented: cannot backfill such sequence operation
HINT: You have attempted to use a feature that is not yet implemented.
See: cockroachdb#42508
```

and

```
> begin;
  alter table t add column y int default nextval('s');
  commit;
pq: transaction committed but schema change aborted with error: (0A000): nextval(): unimplemented: cannot backfill such sequence operation
HINT: You have attempted to use a feature that is not yet implemented.
See: cockroachdb#42508
--
Some of the non-DDL statements may have committed successfully, but some of the DDL statement(s) failed.
Manual inspection may be required to determine the actual state of the database.
--
See: cockroachdb#42061
```

2) The other thing. Prior to this patch, an error "under" a backfill
would be flattened into error code 42P15 (invalid schema), even if
the schema was valid and the error was really about something
encountered during the backfill. This patch fixes it. For example,

```
 ALTER TABLE shopping ADD COLUMN c int AS (quantity::int // 0) STORED
```

Would previously error out with code 42P15, whereas the true error is
22012 (division by zero) which is reported now.

Release note (sql change): CockroachDB will now properly refer to
issue cockroachdb#42508 in the error message hint when a client attempts
to add a sequence-based column to an existing table, which
is an unimplemented feature.

Release note (sql change): CockroachDB will now report an more
accurate error message, hint and error code if/when an error is
encountered while adding a new column.
@knz
Copy link
Contributor Author

knz commented Nov 18, 2019

bors r+

craig bot pushed a commit that referenced this pull request Nov 18, 2019
42509: sqlbase: link to issue #42508 in sequence-related errors r=knz a=knz

(I intend to back-port this to 19.2 and perhaps 19.1.)

Informs #42508.
Informs #42510.

This patch fixes two things.

1) When the backfiller encounters a sequence operation it fails with a
hard error. This and consequences is described further in issue #42508.
To inform the user better, this patch links the error message to that issue.
Additionally, this also ensures that telemetry usage is reported
for the unimplemented feature.

For example:

```
[email protected]:39861/movr> alter table t add column y int default nextval('s');
pq: nextval(): unimplemented: cannot backfill such sequence operation
HINT: You have attempted to use a feature that is not yet implemented.
See: #42508
```

and

```
> begin;
alter table t add column y int default nextval('s');
commit;
pq: transaction committed but schema change aborted with error: (0A000): nextval(): unimplemented: cannot backfill such sequence operation
HINT: You have attempted to use a feature that is not yet implemented.
See: #42508
--
Some of the non-DDL statements may have committed successfully, but some of the DDL statement(s) failed.
Manual inspection may be required to determine the actual state of the database.
--
See: #42061
```

2) The other thing. Prior to this patch, an error "under" a backfill
would be flattened into error code 42P15 (invalid schema), even if
the schema was valid and the error was really about something
encountered during the backfill. This patch fixes it. For example,

```
 ALTER TABLE shopping ADD COLUMN c int AS (quantity::int // 0) STORED
```

Would previously error out with code 42P15, whereas the true error is
22012 (division by zero) which is reported now.

Release note (sql change): CockroachDB will now properly refer to
issue #42508 in the error message hint when a client attempts
to add a sequence-based column to an existing table, which
is an unimplemented feature.

Release note (sql change): CockroachDB will now report an more
accurate error message, hint and error code if/when an error is
encountered while adding a new column.

Co-authored-by: Raphael 'kena' Poss <[email protected]>
@craig
Copy link
Contributor

craig bot commented Nov 18, 2019

Build succeeded

@craig craig bot merged commit adcd2ac into cockroachdb:master Nov 18, 2019
@knz knz deleted the 20191115-errseq branch November 18, 2019 18:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants