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

sql: could not publish new descriptors when creating users within a transaction #107171

Closed
jaylim-crl opened this issue Jul 19, 2023 · 2 comments · Fixed by #108133
Closed

sql: could not publish new descriptors when creating users within a transaction #107171

jaylim-crl opened this issue Jul 19, 2023 · 2 comments · Fixed by #108133
Assignees
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. db-cy-23 T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@jaylim-crl
Copy link
Collaborator

jaylim-crl commented Jul 19, 2023

Describe the problem

When trying to create a SQL user through the CRDB transaction wrapper within cockroach-go, we get the following error: pq: cannot publish new versions for descriptors: [{users 4 1} {role_options 33 1}], old versions still in use.

To Reproduce

Script to reproduce:

package main

import (
	"context"
	"database/sql"
	"fmt"
	"sync"

	"github.com/cockroachdb/cockroach-go/v2/crdb"
	_ "github.com/lib/pq"
)

func main() {
	db, err := sql.Open("postgres", "postgresql://root@localhost:26257/defaultdb?sslmode=disable")
	if err != nil {
		panic(err)
	}
	defer db.Close()

	err = db.Ping()
	if err != nil {
		panic(err)
	}

	ctx := context.Background()
	var wg sync.WaitGroup
	for i := 170; i < 180; i++ {
		wg.Add(1)
		go func(i int) {
			defer wg.Done()
			if err := crdb.ExecuteTx(ctx, db, nil, func(tx *sql.Tx) error {
				query := fmt.Sprintf(`CREATE USER IF NOT EXISTS "user%d"`, i)
				_, err := tx.ExecContext(ctx, query)
				return err
			}); err != nil {
				panic(err)
			}
		}(i)
	}
	wg.Wait()
}

Expected behavior

We expected that the script completes successfully.

Additional data

This issue seems to manifest on v22.2.0 onwards. The error doesn't show up on v22.1.21 (and prior versions).

Environment:

  • CockroachDB version: v22.2.0 onwards
  • cockroach-go SHA: 558bee73bf0634385a862b19708de4ed19316fbb
  • Server OS: MacOS

Jira issue: CRDB-29933

@jaylim-crl jaylim-crl added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jul 19, 2023
@jaylim-crl
Copy link
Collaborator Author

cc: @rafiss

@rafiss rafiss added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Jul 19, 2023
@rafiss
Copy link
Collaborator

rafiss commented Jul 25, 2023

Triage notes:

This cannot publish new versions for descriptors error is supposed to be retryable - in the code it is a twoVersionInvariantViolationError. We should check to make sure we are marking it is retryable correctly. The cockroach-go library already has logic to retry all 40001 errors: https://github.com/cockroachdb/cockroach-go/blob/687b669a01b75ff47d5e74d01f340280e42e5ca0/crdb/tx.go#L204

fqazi added a commit to fqazi/cockroach that referenced this issue Aug 3, 2023
Previously, when release save point was issued for cockroach_restart
this would drive a commit, which could run into retryable errors.
Like the two version invariant error, which is not tagged as a
user facing re triable error, so the client doesn't know to
retry. To address this, this patch converts two version invariant
errors in this case into user facing retryable errors;

Fixes: cockroachdb#107171

Release note (bug fix): Release save point could incorrectly emit
"cannot publish new versions for descriptors" instead of a
retryable error to applications.
fqazi added a commit to fqazi/cockroach that referenced this issue Aug 8, 2023
Previously, when release save point was issued for cockroach_restart
this would drive a commit, which could run into retryable errors.
Like the two version invariant error, which is not tagged as a
user facing re triable error, so the client doesn't know to
retry. To address this, this patch converts two version invariant
errors in this case into user facing retryable errors;

Fixes: cockroachdb#107171

Release note (bug fix): Release save point could incorrectly emit
"cannot publish new versions for descriptors" instead of a
retryable error to applications.
craig bot pushed a commit that referenced this issue Aug 9, 2023
108133: sql: release save point did not retry for two version invariant errors r=fqazi a=fqazi

Previously, when release save point was issued for cockroach_restart this would drive a commit, which could run into retryable errors. Like the two version invariant error, which is not tagged as a user facing re triable error, so the client doesn't know to retry. To address this, this patch converts two version invariant errors in this case into user facing retryable errors;

Fixes: #107171

Release note (bug fix): Release save point could incorrectly emit "cannot publish new versions for descriptors" instead of a retryable error to applications.

108461: sql: fix recently introduced minor bug around PREPARE r=yuzefovich a=yuzefovich

In just merged 4a3e787, we had a minor bug that in case `addPreparedStmt` call fails, we don't restore the original placeholders, which can then lead to panics.

Fixes: #108421.

Release note: None

Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Yahor Yuzefovich <[email protected]>
@craig craig bot closed this as completed in 0a1be2f Aug 9, 2023
blathers-crl bot pushed a commit that referenced this issue Aug 9, 2023
Previously, when release save point was issued for cockroach_restart
this would drive a commit, which could run into retryable errors.
Like the two version invariant error, which is not tagged as a
user facing re triable error, so the client doesn't know to
retry. To address this, this patch converts two version invariant
errors in this case into user facing retryable errors;

Fixes: #107171

Release note (bug fix): Release save point could incorrectly emit
"cannot publish new versions for descriptors" instead of a
retryable error to applications.
fqazi added a commit to fqazi/cockroach that referenced this issue Aug 14, 2023
Previously, when release save point was issued for cockroach_restart
this would drive a commit, which could run into retryable errors.
Like the two version invariant error, which is not tagged as a
user facing re triable error, so the client doesn't know to
retry. To address this, this patch converts two version invariant
errors in this case into user facing retryable errors;

Fixes: cockroachdb#107171

Release note (bug fix): Release save point could incorrectly emit
"cannot publish new versions for descriptors" instead of a
retryable error to applications.
fqazi added a commit that referenced this issue Aug 14, 2023
Previously, when release save point was issued for cockroach_restart
this would drive a commit, which could run into retryable errors.
Like the two version invariant error, which is not tagged as a
user facing re triable error, so the client doesn't know to
retry. To address this, this patch converts two version invariant
errors in this case into user facing retryable errors;

Fixes: #107171

Release note (bug fix): Release save point could incorrectly emit
"cannot publish new versions for descriptors" instead of a
retryable error to applications.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. db-cy-23 T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants