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

multitenant: support admin requests dependent on RangeIterator.Seek #91434

Closed
ecwall opened this issue Nov 7, 2022 · 0 comments · Fixed by #94314
Closed

multitenant: support admin requests dependent on RangeIterator.Seek #91434

ecwall opened this issue Nov 7, 2022 · 0 comments · Fixed by #94314
Assignees
Labels
A-multitenancy Related to multi-tenancy C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)

Comments

@ecwall
Copy link
Contributor

ecwall commented Nov 7, 2022

The following tests fail for secondary tenants with a RangeIterator failed to seek error when SkipSQLSystemTenantCheck is set:

  • ALTER RANGE x RELOCATE LEASE
  • ALTER RANGE RELOCATE LEASE
  • ALTER RANGE x RELOCATE VOTERS
  • ALTER RANGE RELOCATE VOTERS
  • ALTER TABLE x EXPERIMENTAL_RELOCATE LEASE
  • ALTER TABLE x UNSPLIT ALL
  • ALTER INDEX x UNSPLIT ALL

Jira issue: CRDB-21261

Epic CRDB-16746

@ecwall ecwall added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-multitenancy Related to multi-tenancy T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Nov 7, 2022
@ecwall ecwall self-assigned this Nov 7, 2022
craig bot pushed a commit that referenced this issue Nov 8, 2022
90964: streamingccl: avoid passing `evalCtx`, `txn` as parameters to ingestion & replication funcs r=ZhouXing19 a=ZhouXing19

This PR is part of the effort to eliminate usages of `eval.Context.Txn`.
It moves 

1. the definition of `StreamIngestionManager` and `ReplicationStreamManager` under eval;
2. the implementation of `StreamIngestionManager` and `ReplicationStreamManager` via `sql.planner`.

The core changes are 

```
// GetReplicationStreamManager returns a ReplicationStreamManager.
func (p *planner) GetReplicationStreamManager(
	ctx context.Context,
) (eval.ReplicationStreamManager, error) {
	return streaming.GetReplicationStreamManager(ctx, p.EvalContext(), p.Txn())
}

// GetStreamIngestManager returns a StreamIngestManager.
func (p *planner) GetStreamIngestManager(ctx context.Context) (eval.StreamIngestManager, error) {
	return streaming.GetStreamIngestManager(ctx, p.EvalContext(), p.Txn())
}
```

so that the functions under these 2 interfaces run upon `eval.Context` and `kv.Txn` from the `sql.planner`. 

Follow-up: 

- [ ] Pass internal executor from planner too, rather than using `register.ex`.

informs #90923 

91249: acceptance: deflake `TestDockerCLI/test_txn_prompt` r=rafiss a=renatolabs

That test would sometimes fail because of the semantics of `expect` and `send` when the expected string was previously written using `send`.

When `expect` is called, the buffer looked at includes content previously sent using `send`. This means that if one runs `send "foo"; expect foo`, the `expect` call will match instataneously even if the program's output after the send does not contain `foo`.

In the case of the test fixed here, we are supposed to expect for the new prompt to be printed after setting it with `\set prompt1`. In order to properly check whether the prompt changed, this PR changes the prompt `expect` call to use a regular expression that matches the new prompt only if it sits in the beginning of a line.

Prior to this commit, since the `expect` call would return immediately, there was a chance the `send "SET DATABASE"` command could run before the cockroach CLI had the chance to print the new prompt, leading to the following error:

```
abcdefaultdbdef \set prompt1 woo
SET database
woo  ->
.221103 18:13:35.539667600 EXPECT TEST: TIMEOUT WAITING FOR "
 -> "
non-zero exit code: 1
```

Epic: None
Release note: None

91401: spanconfigsqltranslator: add sqlutil.InternalExecutor to SQLTranslator r=arulajmani a=ZhouXing19

This commit is part of the effort of having an internal executor better bound to its outer txn if there's one.

The next step after this commit is to replace the executor used in `s.ptsProvider.GetState()` in `SQLTranslator.Translate()` to the one hanging off `SQLTranslator`.

Informs: #91004

Release note: None

91423: roachpb: fix bug when logging lease in NLE r=ajwerner a=ajwerner

We were logging `lease holder unknown` when the deprecated field was not populated.

Epic: None

Release note: None

91436: multitenant: add admin function `RangeIterator failed to seek` test cases r=rafiss a=ecwall

refs #91434

This change adds test cases for admin functions (see #91434) that fail because of a `RangeIterator failed to seek` error once the multitenant check is bypassed. This needs to be addressed before those admin functions can be supported for secondary tenants.

Release note: None

91508: logictest: fix flake in fk due to sequence non-determinism r=ajwerner a=ajwerner

See [here](https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_BazelEssentialCi/7392167?showRootCauses=false&expandBuildChangesSection=true&expandBuildProblemsSection=true&expandBuildTestsSection=true). This patch stops showing the sequence column while still relying on its ordering properties.

Epic: None

Release note: None

91515: kvserver: return DeprecatedLeaseHolder field in NLHEs r=arulajmani a=arulajmani

v22.1 binaries assume that the leaseholder is unknown when logging NLHE errors if the (Deprecated)LeaseHolder field is unset -- regardless of if the Lease is set or not. We broke this logging in 0402f47 (for mixed version clusters) when we stopped shipping back leaseholder information (in favour of only shipping lease information) on NLHEs. This patch fixes this by populating the (Deprecated)LeaseHolder field when constructing NLHEs.

Release note: None

Co-authored-by: Jane Xing <zhouxing@uchicago.edu>
Co-authored-by: Renato Costa <renato@cockroachlabs.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
Co-authored-by: Evan Wall <wall@cockroachlabs.com>
Co-authored-by: Arul Ajmani <arulajmani@gmail.com>
@ecwall ecwall closed this as completed in dedf02d Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-multitenancy Related to multi-tenancy C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant